Skip to content

feat(Notifications): Adjust alert layout to show more text#1348

Closed
FrazerClews wants to merge 2 commits intoInfiniTimeOrg:developfrom
FrazerClews:frazer/notificationAlertChange
Closed

feat(Notifications): Adjust alert layout to show more text#1348
FrazerClews wants to merge 2 commits intoInfiniTimeOrg:developfrom
FrazerClews:frazer/notificationAlertChange

Conversation

@FrazerClews
Copy link
Copy Markdown

To make better use of the space available in the header, it makes sense to increase the width of the alert_type.

To avoid confusion with the alert_counter, a will separate the alert_counter from the alert_type.

I do have a branch ready which furthers this PR by making better use of the height of the header space as there is a lot of space. This allows more text in the subject content, but this may cause accessibility issues.

before
after

@FrazerClews FrazerClews force-pushed the frazer/notificationAlertChange branch from 0b74a59 to 46b0f18 Compare October 4, 2022 10:03
Copy link
Copy Markdown

@Commenter25 Commenter25 left a comment

Choose a reason for hiding this comment

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

Changes seem fully functional. For anyone wondering, here's a picture of the branch with a shortened header. I personally like it, but I can easily see it feeling cramped.

Screenshot of the branch increasing the notification height

@LinuxinaBit
Copy link
Copy Markdown

LinuxinaBit commented Oct 24, 2022

Is there a way to slightly speed up the text as well?
For long notification titles that would be nice.

@NeroBurner NeroBurner added this to the 1.12.0 milestone Nov 6, 2022
@NeroBurner NeroBurner added the enhancement Enhancement to an existing app/feature label Nov 6, 2022
Copy link
Copy Markdown
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

nice improvement with minimal change, I like it

Improving the scroll speed should be done in a new PR to keep this one minimal

The firmware build is failing because of the branch name containing a / character, the PR itself seems fine

@FrazerClews FrazerClews closed this Nov 6, 2022
@FrazerClews FrazerClews deleted the frazer/notificationAlertChange branch November 6, 2022 21:38
@FrazerClews FrazerClews restored the frazer/notificationAlertChange branch November 6, 2022 21:40
@FrazerClews
Copy link
Copy Markdown
Author

Sorry, accidentally closed this PR trying to change the branch name, made another PR here #1422

@NeroBurner NeroBurner removed this from the 1.12.0 milestone Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement to an existing app/feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants