Edit modal - add accessibility field#3366
Merged
rtibbles merged 8 commits intolearningequality:unstablefrom Apr 19, 2022
Merged
Edit modal - add accessibility field#3366rtibbles merged 8 commits intolearningequality:unstablefrom
rtibbles merged 8 commits intolearningequality:unstablefrom
Conversation
rtibbles
reviewed
Apr 14, 2022
Member
rtibbles
left a comment
There was a problem hiding this comment.
A couple of comments, but this is looking pretty damn close to perfect, code wise!
contentcuration/contentcuration/frontend/channelEdit/components/edit/AccessibilityOptions.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Noting that there's a lag between when a user clicks on the checkbox and when the UI shows that the checkbox is checked. Not sure where this is coming from, but would appreciate any insight here. |
Member
My guess would be that it's an issue with data propagation, and the data not being as optimistically updated as we want. Can take a look locally. |
rtibbles
approved these changes
Apr 19, 2022
Member
rtibbles
left a comment
There was a problem hiding this comment.
Manual testing checks out - the reported input lag seems to be a result of an 18 month old unreported bug, which I will fix in a follow up PR.
This was referenced Sep 15, 2022
Merged
This file contains hidden or 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
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.
Summary
Description of the change(s) you made
Add metadata, strings, tests for accessibility field in the edit modal (
DetailsTabView)Manual verification steps performed
/syncendpoint that the checked items are being sent in astruewhen checked and the key is removed when a checkbox is uncheckedScreenshots (if applicable)
Does this introduce any tech-debt items?
This is the first of the new metadata fields to be included in the edit modal.
Reviewer guidance
How can a reviewer test these changes?
Run the tests:
AccessibilityOptionscomponent usingyarn run test-jest contentcuration/contentcuration/frontend/channelEdit/components/edit/__tests__/accessibilityOptions.spec.jsManually (see also the Gherkin stories for Accessibility here):
/syncendpoint that the checked items are being sent in astruewhen checked and the key is removed when a checkbox is unchecked:References
Contributor's Checklist
Studio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)