Rich text editor | Fix for updating "tabindex" of button in toolbar when clicking on the icon #1503
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
vikisekarNI
approved these changes
Sep 8, 2023
vikisekarNI
reviewed
Sep 8, 2023
m-akinc
approved these changes
Sep 8, 2023
change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json
Outdated
Show resolved
Hide resolved
jattasNI
reviewed
Sep 8, 2023
jattasNI
approved these changes
Sep 11, 2023
rajsite
approved these changes
Sep 12, 2023
|
Since we are in alignment and it's well covered you don't need to wait on @mollykreis's review |
1 task
vivinkrishna-ni
added a commit
that referenced
this pull request
Sep 20, 2023
…alue update in footer buttons (#1550) # Pull Request ## 🤨 Rationale <!--- Provide some background and a description of your work. What problem does this change solve? Include links to issues, work items, or other discussions. --> We recognized that the workaround done in this PR #1503 creates an issue in iOS mobile browsers. In mobile/Tab iOS browsers, when the button is clicked, it does not automatically focus back on the editor, and the keypad closes. Bug link: https://dev.azure.com/ni/DevCentral/_workitems/edit/2526025 With `pointer-events: none` to `slot="start"` of toggle button: https://github.com/ni/nimble/assets/123377523/a0ec33d0-2040-4cc9-ac82-2e8c720f2bba Without `pointer-events: none` to `slot="start"` of toggle button: https://github.com/ni/nimble/assets/123377523/5c4b0e95-5ea4-4248-8559-7450bcdc2d04 After aligning with the leads, as a quicker resolution, we thought that this bug would create more impact for the mobile users than the other `tabindex` issue. ## 👩💻 Implementation <!--- Describe how the change addresses the problem. Consider factors such as complexity, alternative solutions, performance impact, etc. Consider listing files with important changes or comment on them directly in the pull request. --> Reverting all the changes that were implemented here: #1503 ## 🧪 Testing <!--- Detail the testing done to ensure this submission meets requirements. Include automated/manual test additions or modifications, testing done on a local build, private CI run results, and additional testing not covered by automatic pull request validation. If any functionality is not covered by automated testing, provide justification. --> Manually tested iOS devices(mobile and tab) in Safari and Chrome browsers. 1. Opened [nimble.ni.dev](https://nimble.ni.dev/storybook/?path=/docs/incubating-rich-text-editor--docs) on an iOS device and clicked the buttons, the editor loses the focus. 2. Opened the current [storybook publish](https://60e89457a987cf003efc0a5b-kqouhrfikv.chromatic.com/) on an iOS device and clicked the buttons, the editor got the focus as expected. (As shared in the above recordings) ## ✅ Checklist <!--- Review the list and put an x in the boxes that apply or ~~strike through~~ around items that don't (along with an explanation). --> - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. ---------
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.
Pull Request
🤨 Rationale
Partial fix for #1422
Before fix:
tabindexvalue of the icon buttons in the toolbar not getting updated when clicking on the icon.After fix:
tabindexvalue of the icon buttons in the toolbar gets updated when clicked on the icon.With the fix, the last clicked button(anywhere in the button) will get the focus when tabbing to the footer section next time.
👩💻 Implementation
As mentioned in this comment (#1422 (comment)), adding
pointer-events: none;to all the icons used inside thenimble-toggle-button.🧪 Testing
tabindexvalue after clicking on and around the icon in the buttons.tabindexvalue of the button.✅ Checklist