-
Notifications
You must be signed in to change notification settings - Fork 687
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
Replace unwrapped "up" and "down" strings used for aria labeling #12817
base: develop
Are you sure you want to change the base?
Replace unwrapped "up" and "down" strings used for aria labeling #12817
Conversation
Build Artifacts
|
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.
Hi @AllanOXDi - overall this is a solid start. Adding in the validation based on Richard's recommendation is great. I know we may still change some of the wording for the strings, but making sure that they are all translated strings, and making sure that all of the functions are returning strings properly, is important.
I know there are quite a few places where this draggable
with up and down is used, but could you please run your local dev server, with the console open, and go to view each changed file, and resolve any console errors? I've added one example inline of a place where I can tell that there will be an issue, but I'd like you to go through each file, look at the console, and fix any problems. It shouldn't take long, but it's blocking. Thank you!
kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/CreateQuizSection.vue
Outdated
Show resolved
Hide resolved
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.
Blocking:
- Capitalize the message strings
Non-blocking:
- I am conflicted about passing a Function that wraps a string. I love the idea of having a validation for passing along translated strings, but it is an unfamiliar approach.
- I wonder if we should export your
isWrappedString
validator from thekolibri.utils.i18n
library so that we could start using it elsewhere. - Namely this would be neat to somehow use with KDS as we pass many strings to those components.
Overall though this looks great - I think fixing Marcella's change request and capitalization should make this ready to go.
kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionOrder.vue
Outdated
Show resolved
Hide resolved
kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionOrder.vue
Outdated
Show resolved
Hide resolved
kolibri/plugins/device/assets/src/views/RearrangeChannelsPage.vue
Outdated
Show resolved
Hide resolved
58ec872
to
007de86
Compare
eceef01
to
c04562d
Compare
56f998f
to
3648fc5
Compare
dd36619
to
d3b8251
Compare
c0a124a
to
b1ced14
Compare
c801dda
to
1e4a4b1
Compare
Summary
This PR audits and replaces unwrapped "up" and "down" strings used for aria labeling
References
#12392
Reviewer guidance.
review the slack conversation here for more context
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)