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

Add disable parameter to keyboard widget #8223

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pmario
Copy link
Member

@pmario pmario commented May 29, 2024

This PR is related to PR: use a textarea in advanced-search-filter tab #8194 which should be implemented differently in wikitext.

To be able to implement #8194 functionality as a plugin the core needs to support a "disabled" parameter for the keyboard-widget, which this PR implements.

  • "disabled" defaults to "no"
  • If set to "yes" the keyboard widget event-handler will not be created in the DOM.
  • If disabled a tc-disabled class is assigned to the HTML element, so it can be used for styling if needed.

Copy link

vercel bot commented May 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview May 29, 2024 0:13am

@saqimtiaz
Copy link
Member

To be able to implement #8194 functionality as a plugin the core needs to support a "disable" parameter for the keyboard-widget, which this PR implements

Can you explain why this is necessary, as opposed to just not using the keyboard-driven-input macro in the UI that the plugin implements?

@pmario
Copy link
Member Author

pmario commented May 29, 2024

The "keyboard-driven-input" macro also implements the CTRL-ALT-left and CTRL-ALT-right shortcuts to navigate between the tabs. This functionality should still work.

If the filter-input is a text area, the up and down keys are needed to navigate inside the text area, so they need to be disabled within the keyboard-driven-input macro.

@Jermolene
Copy link
Member

The "keyboard-driven-input" macro also implements the CTRL-ALT-left and CTRL-ALT-right shortcuts to navigate between the tabs. This functionality should still work.

If the filter-input is a text area, the up and down keys are needed to navigate inside the text area, so they need to be disabled within the keyboard-driven-input macro.

Thanks @pmario I am tempted to suggest that we remove the CTRL-ALT-left and CTRL-ALT-right shortcuts from advanced search, and instead try to implement keyboard shortcuts in a general purpose way within the tabs macro. It certainly complicates the advanced search code a lot.

@saqimtiaz
Copy link
Member

@pmario please note that the previous and next tab behaviour is independent of the keyboard-driven-input(kdi) macro, so you can maintain that behaviour and not use the kdi macro.

See: https://github.com/Jermolene/TiddlyWiki5/blob/3af2a0ae6f95cbe76381ab1125f4a29632cf4178/core/ui/AdvancedSearch/Filter.tid#L6-L13

@pmario
Copy link
Member Author

pmario commented May 30, 2024

I think the disabled parameter for the keyboard widget is useful independent of the keyboard-driven-input macro. It can help to make macros more flexible, in the same way as it does work for the button-widget

@saqimtiaz
Copy link
Member

@pmario The genesis widget already offers a great deal of flexibility in terms of conditionally creating a keyboard widget. Can you think of a scenario where that is not sufficient or flexible enough?

@pmario
Copy link
Member Author

pmario commented May 30, 2024

The genesis-widget is OK for advanced users to extend the functionality of core widgets, but users should not be forced to use it, to fix deficits of core widgets.

@saqimtiaz
Copy link
Member

saqimtiaz commented May 30, 2024

I don't see any deficit here with the keyboard widget, and the genesis widget is precisely the tool that should be used when desiring flexibility as to which widget/element to create. This was one of the driving factors behind its creation. It is not meant just for overriding or tweaking core widgets.

Furthermore disabling the keyboard widget in this manner is extremely awkward for a widget that usually has no visible user affordance and this has no precedence, neither in the core nor in HTML.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants