GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Notification -> Notifications by tituscmd · Pull Request #2238 · InfiniTimeOrg/InfiniTime

Conversation

@tituscmd

@tituscmd tituscmd commented Jan 27, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This PR is a small change and it might very well be up to preference.

In the little notifications screen above the watchface, the "Notification" as well as "No notification to display" is now changed to "Notifications" and "No notifications to display".

In my opinion using the plural makes more sense here, since the screen can show more than just one notification.
Feel free to tell me what you think!

Here's a pic of how it looks like in InfiniEmu:
Screenshot_2025-01-27_17-21-01

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
marigoldfish, pipe01, and NitroNils reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
tituscmd and others added 3 commits January 21, 2025 13:50
@tituscmd
Merge pull request #26 from InfiniTimeOrg/main
c73f016 
Alarm: Simplify alarm alerting screen (InfiniTimeOrg#2211)
@tituscmd
changed it from 'Notification' to 'Notifications'
5ef319a
@tituscmd
Merge branch 'main' into notificationS
4051782
@github-actions

github-actions Bot commented Jan 27, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Build size and comparison to main:

Section Size Difference
text 373024B 16B
data 948B 0B
bss 22536B 0B

Run in InfiniEmu
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
FintasticMan
FintasticMan approved these changes Jan 27, 2025
View reviewed changes

@FintasticMan FintasticMan left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow, I haven't noticed this before!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveAmor

SteveAmor commented Jan 27, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I read the singular as:
Notification 0 of 0
1 of 1
1 of 2
.....
.....
4 of 5
etc, etc.

I also read the specific case for Notification 0/0 as the notification that you are currently on so the area which would normally display the notification singularly says (correctly in my opinion) "No notification to display".

It's only my opinion and would save 16B.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tituscmd
Merge branch 'main' into notificationS
450c66f
@tituscmd

tituscmd commented Feb 3, 2025

Copy link
Copy Markdown
Contributor Author

I agree that either "Notification" or "Notifications" is in some way not perfectly descriptive. Something like "Notification Center" would be ideal, but it doesn't fit inside that space.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mark9064 mark9064 added enhancement Enhancement to an existing app/feature UI/UX User interface/User experience labels Feb 7, 2025
mark9064
mark9064 approved these changes Feb 10, 2025
View reviewed changes

@mark9064 mark9064 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also agree that this isn't optimal, but that it fits better with the s than without. May as well merge this for now, and then if someone suggests a better idea later on we can switch to it

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mark9064
mark9064 merged commit d371ebc into InfiniTimeOrg:main Feb 10, 2025
@mark9064 mark9064 added this to the 1.16.0 milestone Feb 10, 2025
dariusarnold pushed a commit to dariusarnold/InfiniTime that referenced this pull request Mar 2, 2025
@tituscmd @dariusarnold
Improve no notification text (InfiniTimeOrg#2238)
333ca34 
Change the "No notification" text to "No notifications"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mark9064 mark9064 mark9064 approved these changes
@FintasticMan FintasticMan FintasticMan approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

enhancement Enhancement to an existing app/feature UI/UX User interface/User experience

Projects

None yet

Milestone

1.16.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@tituscmd @SteveAmor @mark9064 @FintasticMan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.