-
Notifications
You must be signed in to change notification settings - Fork 1.4k
simplify raw tag editor ui and logic #10889
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
Conversation
f24bcb4 to
43940c6
Compare
43940c6 to
97de8b9
Compare
| .call(reference.button); | ||
| .call(reference.button) | ||
| .select('.tag-reference-button') | ||
| .attr('tabindex', -1) |
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.
skip delete and tag info buttons while navigating using tab: makes it more quick to get to where one typically needs to, deleting a key using the keyboard only is possible by emptying the tag's key, and advanced users are not typically using the i button anyway
I’ll definitely appreciate not having to tab past the 🗑️ and ℹ️ buttons. But keyboard navigation is not just for advanced users; it’s also for people who need to use the keyboard more often for ergonomic reasons. Advanced users probably do use the ℹ️ button too, though maybe not as often. On the other hand, this would make the raw tag editor more consistent with the tab order in the fields.
GitHub has some controls (such as the code editor) that co-opt the Tab key but allow Tab to do the normal thing with a modifier key or after pressing Escape. Maybe we can come up with a key combination that always toggles the ℹ️ when either a field or a raw tag is focused?
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.
key combination that always toggles the ℹ️
That's an awesome suggestion! As this shortcut would be typically pressed while the cursor is in an input box, it must be a shortcut with a modifier key. Something like Ctrl + ? could work…
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.
Something like
Ctrl+?could work…
Unfortunately that particular shortcut could cause problems in some keyboard layouts and operating systems. The equivalent to Ctrl? on macOS would be ⇧⌘?, which in the U.S. English layout would be ⇧⌘/ just like the system-wide shortcut for searching menu items. I’m guessing the most obvious key combinations for help content have already been taken, but it’s OK if the key combination is obscure, as long as it exists.
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.
On macOS, there’s a system-wide Full Keyboard Access setting for whether the tab order only includes text input controls or whether it includes every user input control. Regardless of this setting, holding down Option causes the Tab key to include every user input control, at least in Safari. Ideally we’d make sure this functionality still works for those who rely more heavily on the keyboard for navigation.
| .merge(list); | ||
|
|
||
|
|
||
| // Container for the Add button |
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.
replaces + button and tab logic
The blank row is an elegant approach, but will removing this button make the remaining ➕ button in the relations section less intuitive? I think there’s probably already some initial confusion over whether clicking it creates a new relation or adds to an existing relation. The analogue to this change would be to always show an empty relation membership row.
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.
Good idea, definitely worth experimenting with.
* always show empty line at bottom of the tag list to allow adding more tags (replaces `+` button and tab logic) * disable delete and info buttons on empty tag * replace deferring of tag change events and re-rendering with a simpler logic (to fix issues like #10871): only set input field values if the rendering event was triggered by an external event, or by a change of the respective field in the raw tag editor itself * skip delete and tag info buttons while navigating using tab: makes it more quick to get to where one typically needs to, deleting a key using the keyboard only is possible by emptying the tag's key, and advanced users are not typically using the `i` button anyway
97de8b9 to
c27c5b2
Compare
+button and tab logic)ibutton anywaytodo: