[tabindex] use tab to cycle through inputs #3302

BurningTreeC · 2018-05-21T06:24:10Z

This sets the tabindex of buttons and links to "-1" by default so that they get ignored when cycling using TAB - see issue #3298

Sets the tabindex of <input> and <textarea> elements to "0" by default so that it uses the DOM-structure order when cycling

With this, one can jump from the title field directly to the tag input field, from the tag input field directly to the text field, from there directly to the type field, to the field name, to the field value ...

custom tabindices for buttons, links and input fields/areas can be set by passing the tabindex as an attribute

the type input-field already cancels its popup automatically when going ahead using TAB
the popups in the tag input-field and the field-name input-field don't get cancelled - I think they should

keeps buttons from focusing by using TAB if no tabindex is given

keeps links from focusing by using TAB

this passes the tabindex to simple.js/framed.js/codemirror-engine

this allows TAB to cycle through input fields in the order of the DOM structure when no other tabindex is specified sets tabindex "0" by default

BurningTreeC · 2018-05-21T07:50:23Z

Now the engines cancel popups on focus before they trigger new popups

BurningTreeC · 2018-05-21T08:10:30Z

Now I don't know how you want to handle tabindices of [[]] and [ext[]]links
I'm happy about advice

BurningTreeC · 2018-05-21T08:23:42Z

I could also address #3237 with this PR
Not sure why "+14px" for the iframe height stands there, maybe it was relevant up to some point in time?
Omitting that seems to restore correct behaviour but I may be wrong

Jermolene · 2018-05-21T09:15:21Z

Hi @BurningTreeC

Not sure why "+14px" for the iframe height stands there, maybe it was relevant up to some point in time?

Annoyingly, I didn't leave enough crumbs to figure out my original thinking, but it would certainly be good to sort it out. It's one of those cases where we need to take great care to test in all browser configurations (the fudge factor could have been to prevent scroll bars appearing, for instance).

BurningTreeC · 2018-05-21T10:04:58Z

It's one of those cases where we need to take great care to test in all browser configurations

I'm not able to test in all browsers. We should create another PR for that

Jermolene · 2018-05-21T10:16:05Z

Hi @BurningTreeC

I'm not able to test in all browsers. We should create another PR for that

I develop on macOS and so can easily test in Firefox, Safari, Chrome and Opera. I use Windows virtual machines to test IE and Edge (the VMs are free these days from MS). Depending on your OS you may not be able to cover as many browsers as that but it is important that we test UI-impacting changes as broadly as possible. If you're not able to achieve broad coverage yourself then make a note in the PR; I tend to assume that contributors have tested PRs (I'm an optimist 😄)

BurningTreeC · 2018-05-21T12:46:03Z

I tend to assume that contributors have tested PRs (I'm an optimist)

pretty optimistic 😁

setting up vm's is no big deal - but for macOS it is if you don't have the license.
I test on chrome, chromium, TD, firefox old and new, qutebrowser (webkit engine), beaker, SOMETIMES edge and ie if I have the feeling that it's needed. I have a windows install on another partition and will try to be less careless about them.

That said, you can assume that I don't test Safari..

When I have time I will take a look at this 14px thing on various browsers.

If someone else does, even better 👍

But I'd like to move it back to the original issue (#3237) because it apparently needs more than just removing it

BurningTreeC · 2018-05-22T07:20:12Z

all input widgets and the link widget now have their tabindices:

"0" by default for textarea and <input> tags, so TAB cycles through them in the order of the DOM
"-1" by default for all other inputs - buttons, checkboxes, range, select, link..

[[]] and [ext[]] links are not yet changed

I'd like to ask @Jermolene @pmario first what you prefer as default tabindex
"-1" makes the elements unfocusable which may be bad for assistive technology

BurningTreeC · 2018-05-22T07:54:44Z

... making all links, buttons, checkboxes etc tabindex="1" will first focus all inputs and textareas (tabindex="0") and then go to those with tabindex 1

we could even set an index on the tiddler frames to navigate from a tiddler to the next with TAB and back with Shift-TAB

I don't know how to proceed because of these decisions and will wait with this

pmario · 2018-05-22T07:57:24Z

core/modules/editor/engines/framed.js

+	if(this.widget.editTabIndex) {
+		this.iframeNode.setAttribute("tabindex",this.widget.editTabIndex);
+	} else {
+		this.iframeNode.setAttribute("tabindex","0");


I'm not happy!

domNode.setAttribute() is an "expensive" function. ... So if we can avoid to call it, we definitely should. The else path now makes this action mandatory for every dom element, even if a user doesn't use it.

personal rant ;)

The point is. I personally don't care about the tab order, because it will always be wrong. ... And using the "tab-key" to navigate input elements is similar to the "original sin" introduced by MS.

Ok, the original cause of this is that one can jump from the title field to the tag input to the text editor with TAB without focusing the buttons in between

domNode.setAttribute() is an "expensive" function. ... So if we can avoid to call it, we definitely should

Ok

The else path now makes this action mandatory for every dom element, even if a user doesn't use it.

why for every dom element?

why for every dom element?

ok. every DOM element that can get input focus. ...

eg: Links already create the biggest UI stutter we have at the moment. Just switch between the shadow and system sidebar tabs. IMO this change will make it worse.

I wanted to discuss if tabindices should or should not be set for links, Buttons and so on …
I am very fine with NOT touching them, reverting my changes on those Elements and only giving the Input and text-Areas their Indices so that TAB works for Jumping between them

… do you think calling setAttribute() within the simple and framed engines is too expensive or are you more concerned About the amount of links and Buttons (for whom I'll most likely remove the tabindex again)?

… do you think calling setAttribute() within the simple and framed engines is too expensive or are you more concerned About the amount of links and Buttons (for whom I'll most likely remove the tabindex again)?

I did comment on the first else ... setAttribute() line Iine that I saw, because there are several. ...

I'm concerned about the number of elements that are affected by the change.

eg: If you open the right sidebar on tiddlywiki.com and try navigate the TOC structure with the keyboard only! IMO the existing experience is completely broken. ...

Will your PR fix this? If yes good. .... If it only improves the tiddler edit mode, it should be only active in the tiddler edit form and nowhere else. ...

Just my thoughts.

Will your PR fix this? If yes good. ....

To know that, I'd need to know how it should behave

I'm concerned about the number of elements that are affected by the change.

your concerns made me see this from a different view. I'm in favor of making this for tiddler edit forms only...

How to check if the input is within a tiddler? Is this.getVariable("storyTiddler") !== undefined good enough?

Note that I'm going to remove the tabindex for buttons, links, checkboxes etc. and leave it for input and textarea tags only - there I'd check then if we're within a tiddler

BurningTreeC · 2018-05-22T15:36:08Z

@pmario @Jermolene

Respecting the concerns Mario mentioned I have made a version that sets the tabindex for inputs and textareas within tiddlers only (where storyTiddler is defined)
That could also be done explicitly by setting an attribute like setTabIndex, do you think that's better?

I'm already setting an attribute cancelPopups on the title, tag, text, type and fields inputs/textareas (in the corresponding EditTemplates only) which cancels previous popups when those input areas get focus, so that one doesn't have to click those popups (tag dropdown, type dropdown, others) away.

When this decision is made, I'd like to close this and make a new PR with this simpler version

Simon

pmario · 2018-05-22T17:12:38Z

I did test your code in tiddler edit mode, as it is at the moment. ... The behaviour there seems to be cleaner as with the 5.1.17 at tiddlywiki.com. ... So I think I'm ok with the changes atm.

We still have the problem, that we can't navigate the dropdown elements. But that's probably a completely different topic.

BurningTreeC · 2018-05-22T17:50:23Z

I did test your code in tiddler edit mode, as it is at the moment. ... The behaviour there seems to be cleaner as with the 5.1.17 at tiddlywiki.com

Thanks @pmario - I'm not going to change that behaviour in edit mode, but you were right with the tabindices on all other elements where it's unnecessary

I've seen that it's a delicate tweak and therefor I want to get it right for the original issue first and that's the tiddler in edit mode, jumping from input to input

The dropdown items on tiddlywiki 5.1.17 can be focused using tab because input fields and dropdown items have the same default tabindex and the dropdown items come after the input in the DOM structure

Now, if I set all inputs and textareas in an edit-tiddler to "1" (I'm testing 1 for inputs locally atm) I can jump between the inputs but the items with lower index-level get "overjumped" as in my current code

That, I think, is a new issue that comes with solving the other and we need to decide if we solve the original issue accepting the unfocusable dropdown items or not. I like the new version - I either write tags, type and fieldnames or select them by mouse

pmario · 2018-05-22T20:23:53Z

That, I think, is a new issue that comes with solving the other and we need to decide if we solve the original issue accepting the unfocusable dropdown items or not. I like the new version - I either write tags, type and fieldnames or select them by mouse

Yea, I think the new behaviour is better, since the old one opens several dropdowns but didn't close them anymore. And they can't be selected with tabs anyway.

BurningTreeC · 2018-05-23T07:01:58Z

Yea, I think the new behaviour is better, since the old one opens several dropdowns but didn't close them anymore. And they can't be selected with tabs anyway.

I think so, too
And clicking into textareas cancels popups, too. Currently one has to click somewhere outside to cancel a popup. Two birds with one stone

I've made a simpler PR - #3309
It behaves the same for edit-mode-tiddlers but leaves all links and buttons and friends untouched

pmario · 2018-05-23T08:05:24Z

so you may close this one?

BurningTreeC · 2018-05-23T08:22:43Z

so you may close this one?
yep!

BurningTreeC added 8 commits May 21, 2018 08:01

set tabindex -1 if no index specified

be72ff1

keeps buttons from focusing by using TAB if no tabindex is given

set tabindex -1 if no index specified

61b8a6b

keeps links from focusing by using TAB

get tabindex attribute in factory.js

b8b7cf4

this passes the tabindex to simple.js/framed.js/codemirror-engine

set tabindex 0 for simple inputs

886bbf6

this allows TAB to cycle through input fields in the order of the DOM structure when no other tabindex is specified sets tabindex "0" by default

sets tabindex 0 for framed inputs if not specified

ddae714

framed engine - cancel popups on focus

4427777

simple engine - cancel popups on focus

4530d93

cm engine - cancel popups on focus

ac49042

BurningTreeC mentioned this pull request May 21, 2018

<TAB> to cycle through title-tag-text-type-fields inputs #3298

Closed

BurningTreeC added 4 commits May 21, 2018 09:57

Update ButtonWidget.tid

177ce0a

Update LinkWidget.tid

310058f

Update ButtonWidget.tid

777fdd5

bad typo

08a5eca

BurningTreeC mentioned this pull request May 21, 2018

Editor height bleeds outside iframe #3237

Closed

BurningTreeC added 5 commits May 22, 2018 08:51

tabindex for browse element

268a7d9

tabindex for checkbox

bbf33be

tabindex for radio input

4c74419

tabindex for range widget

a60e655

tabindex for select widget

7352d6e

BurningTreeC added 2 commits May 22, 2018 09:26

default indices '1' for buttons etc, '0' for inputs and textareas only

f6c60ab

tabindex must be set on iframe

805e986

pmario reviewed May 22, 2018

View reviewed changes

BurningTreeC mentioned this pull request May 23, 2018

[Tabindex] input navigation with TAB #3309

Closed

BurningTreeC closed this May 23, 2018

BurningTreeC deleted the patch-25 branch June 23, 2019 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tabindex] use tab to cycle through inputs #3302

[tabindex] use tab to cycle through inputs #3302

BurningTreeC commented May 21, 2018 •

edited

Loading

BurningTreeC commented May 21, 2018

BurningTreeC commented May 21, 2018

BurningTreeC commented May 21, 2018

Jermolene commented May 21, 2018

BurningTreeC commented May 21, 2018

Jermolene commented May 21, 2018

BurningTreeC commented May 21, 2018 •

edited

Loading

BurningTreeC commented May 22, 2018

BurningTreeC commented May 22, 2018

pmario May 22, 2018 •

edited

Loading

BurningTreeC May 22, 2018

BurningTreeC May 22, 2018

pmario May 22, 2018

BurningTreeC May 22, 2018

BurningTreeC May 22, 2018

pmario May 22, 2018

BurningTreeC May 22, 2018

BurningTreeC commented May 22, 2018

pmario commented May 22, 2018

BurningTreeC commented May 22, 2018

pmario commented May 22, 2018

BurningTreeC commented May 23, 2018

pmario commented May 23, 2018

BurningTreeC commented May 23, 2018

[tabindex] use tab to cycle through inputs #3302

[tabindex] use tab to cycle through inputs #3302

Conversation

BurningTreeC commented May 21, 2018 • edited Loading

BurningTreeC commented May 21, 2018

BurningTreeC commented May 21, 2018

BurningTreeC commented May 21, 2018

Jermolene commented May 21, 2018

BurningTreeC commented May 21, 2018

Jermolene commented May 21, 2018

BurningTreeC commented May 21, 2018 • edited Loading

BurningTreeC commented May 22, 2018

BurningTreeC commented May 22, 2018

pmario May 22, 2018 • edited Loading

Choose a reason for hiding this comment

BurningTreeC May 22, 2018

Choose a reason for hiding this comment

BurningTreeC May 22, 2018

Choose a reason for hiding this comment

pmario May 22, 2018

Choose a reason for hiding this comment

BurningTreeC May 22, 2018

Choose a reason for hiding this comment

BurningTreeC May 22, 2018

Choose a reason for hiding this comment

pmario May 22, 2018

Choose a reason for hiding this comment

BurningTreeC May 22, 2018

Choose a reason for hiding this comment

BurningTreeC commented May 22, 2018

pmario commented May 22, 2018

BurningTreeC commented May 22, 2018

pmario commented May 22, 2018

BurningTreeC commented May 23, 2018

pmario commented May 23, 2018

BurningTreeC commented May 23, 2018

BurningTreeC commented May 21, 2018 •

edited

Loading

BurningTreeC commented May 21, 2018 •

edited

Loading

pmario May 22, 2018 •

edited

Loading