Improve notifications settings #12497
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brawaru
commented
Nov 28, 2019
config/locales/en.yml
Outdated
| @@ -580,6 +580,10 @@ en: | |||
| discovery: Discovery | |||
| sensitive_content: Sensitive content | |||
| toot_layout: Toot layout | |||
| notifications: | |||
| email_events: Events for e-mail notifications | |||
| email_events_hint: Select events that you want to receive notifications for | |||
There was a problem hiding this comment.
This one clearly missing a termination! If you are reviewing this, please tell: should I use colon (:) here or just dot (.) is okay?
|
I also missed to normalize localization files, that's why CI failed. Fixed. Previous question is still active though. Whenever it resolves, I will squash the commits (hope this is okay). UPD: (stares at CI status) “What is wrong with you? >_>” |
ykzts
requested changes
Nov 29, 2019
Currently notifications page seems a bit cluttered with no clear separation between e-mail and filtering settings. This commit tries to address them by adding clear separation with headers, hints and removing continuously reused texts for events checkboxes.
ykzts
approved these changes
Nov 29, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Currently notifications page seems a bit cluttered with no clear separation between e-mail and filtering settings. This PR tries to address them by adding clear separation with headers, hints and removing continuously reused texts for events checkboxes.
Visual Comparison:
(click to see full sized pictures)
Warning about visuals
Please note, that due to my technical inability to set up Mastodon instance to test these changes, screenshots above were made with browser's developer tools, using the texts from files and placing new elements at expected places. If that does not satisfy you, you may set up your own instance and review changes manually.
A word about PR
I'm nowhere familiar with what “HAML” is and not explored it yet, because this commit is so small, the changes primarily were made by reviewing other files in project and guessing the syntax. I expect maintainers (or reviewers) to test it and suggest what to fix and how. Thanks!