Rich text editor | Reverting back the workaround for the "tabindex" value update in footer buttons #1550
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 19, 2023
|
We are making this PR ready for review ahead of a buddy review, as it involves the removal of the workaround code. |
m-akinc
approved these changes
Sep 19, 2023
There was a problem hiding this comment.
It's questionable to me that we would revert a general fix for the sake of the mobile experience. My understanding is that mobile support has been deprioritized, so we should not bother fixing those kinds of issues. However, the workaround being reverted does not seem super important (and we're still tracking the issue), so I'm not strongly opposed to this change.
change/@ni-nimble-components-b0a59426-c9a2-444a-a9f7-1ccf79dd94b4.json
Outdated
Show resolved
Hide resolved
jattasNI
approved these changes
Sep 19, 2023
rajsite
approved these changes
Sep 19, 2023
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
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: nonetoslot="start"of toggle button:ios-focus-issue.mp4
Without
pointer-events: nonetoslot="start"of toggle button:ios-focus-fix.mp4
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
tabindexissue.👩💻 Implementation
Reverting all the changes that were implemented here: #1503
🧪 Testing
Manually tested iOS devices(mobile and tab) in Safari and Chrome browsers.
1. Opened nimble.ni.dev on an iOS device and clicked the buttons, the editor loses the focus.
2. Opened the current storybook publish on an iOS device and clicked the buttons, the editor got the focus as expected. (As shared in the above recordings)
✅ Checklist