Skip to content
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

[Tabindex] input navigation with TAB #3309

Closed
wants to merge 27 commits into from

Conversation

BurningTreeC
Copy link
Contributor

@BurningTreeC BurningTreeC commented May 23, 2018

Follow-Up of #3302

This now ONLY sets the tabindices of inputs and textareas within tiddlers to 1

Why 1?
Because the other elements have "0" which is default and the title gets focused automatically or by clicking, and TAB will then first cycle through all "1-level" elements and then jump to the base-level (or level 2 first [3 ... 4 ...], if it exists - but it doesn't here)

The inputs and textareas of the EditTemplate tiddlers all get the new attribute cancelPopups that cancels all existing popups when they get focus (and then spawn their popup if so). Cannot set this on all inputs by default because of the link-dropdown in the toolbar which would cancel itself after opening.
We could set cancelPopups on the search input, too - it'd be very comfortable there, too

The tabindex currently is hard-coded within simple.js and engine.js - should we pass it as a widget-attribute?

The check for "is the input within a tiddler?" may be undesired because it affects the ViewTemplates, too.
Simply passing say setTabIndex="1" explicitly would be a possible solution and would allow to control these changes in a sane way.

What do you think?

@BurningTreeC
Copy link
Contributor Author

passing tabIndex as attribute for the edittemplate inputs only (including codemirror)
$:/config/EditTabIndex can be used to change those tabindices back to "0"

this can be tested

BurningTreeC added 2 commits May 23, 2018 22:29
@ldorigo
Copy link
Contributor

ldorigo commented May 28, 2018

Yes, please! I've been wishing for this for a while. It's a first step to making TW more keyboard-navigable.

@pmario
Copy link
Member

pmario commented Jun 7, 2018

We could set cancelPopups on the search input, too - it'd be very comfortable there, too

+1

@BurningTreeC
Copy link
Contributor Author

Hi @pmario , thanks for the thumbs-up!

I wanted to ask you about a codemirror-config detail:

here is a codemirror issue with a solution for disabling the TAB key, which would enable TAB to jump from a codemirror editor to next/previous input. I'd like to make that configurable in the ControlPanel (and keep the default CM behaviour as default).
It would need to set the index "Tab" in "extraKeys" to false (boolean).

Do you have an idea how to add this config? I've not been lucky trying

@BurningTreeC
Copy link
Contributor Author

@Jermolene, this one would be ready, I reopened it.

This does the following:

  • cancels popups when focusing an input that has the attribute cancelPopups="yes" - then if the input has a popup, it gets spawned
  • set tab-indices for the title, tags, editor, type and fieldname / fieldvalue inputs so that we can Tab and shift-Tab through them

@Jermolene
Copy link
Member

Thanks @BurningTreeC does this undo #3490 or does that need to be done separately?

@BurningTreeC
Copy link
Contributor Author

@Jermolene , this does not undo #3490

@BurningTreeC
Copy link
Contributor Author

More detailed explanation what this PR is supposed to achieve:


This PR adds two attributes that can be used for the edit-text and edit widgets: cancelPopups and tabIndex

cancelPopups="yes" will cancel open popups when the input gets focus so that it also works if we focus an input using Tab for navigating to it

tabIndex lets us assign a tab-index attribute to the input, so that we can create input sequences that are accessible with Tab and shift-Tab without focusing other elements like buttons that may be in between the input fields

the default tab-index for elements without an explicitely set tab-index attribute is 0
#3309 sets the tab-index for the 6 default input fields of the EditTemplate only to 1
that means that if an element with tab-index level 1 has focus, Tab will cycle through all elements with level 1 first, then look for higher levels and if there are no higher levels it goes back to level 0

I'm using attributes here, because we don't want input fields that are within popups to cancel their own popup. Maybe we need a logic that #3490 introduced, thus, detecting if the input is within the popup that we're trying to cancel

@hoelzro
Copy link
Contributor

hoelzro commented Nov 27, 2018

@BurningTreeC Nice work! I have some thoughts on this:

https://github.com/Jermolene/TiddlyWiki5/blob/513853614803726632dbe17d017851341ec516a0/core/modules/editor/engines/framed.js#L160-L165

For what it's worth, a single $tw.popup.cancel(0) will cancel all currently active popups.

To be honest, I feel a little uneasy about the explicit cancelPopups property - it feels a little like "action at a distance" to me, and it's something that everyone will need to include in all of their wikitext containing editors if they want consistent behavior with popups. I think that the popup mechanism should be smart enough to manage the lifecycle of popups (and it should hook into click and focus events to cancel popups when appropriate). Perhaps I should restart the discussion in #3484 (or start a fresh one on Google Groups) around the needs and constraints of the popup system!

@BurningTreeC
Copy link
Contributor Author

@hoelzro I think you're right about the popup mechanism

I'll remove any popup handling from this PR and leave the tabindex only

@BurningTreeC
Copy link
Contributor Author

Now only the tab-index attribute is part of this PR

let's look into #3484 and #3490 for popup-handling (cancelling)

@BurningTreeC
Copy link
Contributor Author

closing this in favor of a clean PR

@BurningTreeC BurningTreeC deleted the patch-27 branch June 23, 2019 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v5.1.20 Planned for v5.1.20
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants