-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Announcement bar] Change auto rotate announcements copy and min value #2787
Conversation
}, | ||
"change_slides_speed": { | ||
"label": "Change slides every" | ||
"label": "Change every" |
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.
Localization quality issue found
The following issues may affect the quality of localized translations if they are not addressed:
- The value
Change every
for keysections.announcement-bar.settings.change_slides_speed.label
is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.
Please look out for other instances of this issue in your PR and fix them as well if possible.
Questions about these messages? Hop in the #help-localization Slack channel.
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 updated the screenshot to share more context around that change 👍
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.
This looks good to me! I tested the following things and didn't find a way to break anything
- Test the automatic slide on desktop version
- Tested on mobile to ensure that the announcement bar doesn't rotate.
- Added multiple announcement bars to ensure nothing broke
- Test in safari and chrome
- Applied color scheme to announcement bar to ensure nothing was broken
- Checked and unchecked the Auto-rotate on desktop
- tested with the new 3s min.
- Tested with varying speeds
- Added social icons to the announcement bar and tested with different speeds
I tried a couple different scenarios and wasn't able to find a way to break this! The code looks good to me and I am liking the label changes! They are user friendly!
Shopify#2787) * Change auto rotate announcements copy and min value * adjust copy based on feedback * Update 4 translation files * Update 14 translation files * Update 2 translation files --------- Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
PR Summary:
Why are these changes introduced?
Fixes #2785
What approach did you take?
Change the copy of the text and updated the
min
value. Though I wonder if we should change thestep
. Slideshow has2
for step while announcements have1
. But that'd potentially break folks current setup if they used6s
and can now only have5s
or7s
though it doesn't super impactful.Other considerations
Decision log
Visual impact on existing themes
None, should only be a new setting option (3s) and content change.
Testing steps/scenarios
Demo links
Checklist