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

[UI] add selectable tags- and search-dropdown #3565

Closed
wants to merge 59 commits into from

Conversation

BurningTreeC
Copy link
Contributor

@BurningTreeC BurningTreeC commented Nov 21, 2018

this adds a saveTiddler attribute to factory.js

if it's present it's used as a tiddler title where all the inputs/changes made by the user get stored
this can be used for selecting dropdown-entries that are filterable by an input field
the entries should be sorted by the saveTiddler and the original tiddler (<$edit-text tiddler="originaltiddler" .../>) can be updated with the selected dropdown-entry which will be shown within the input, without changing the filtering of the dropdown-items


Update: to the above mentioned changes, this adds:

  • a refreshTiddler attribute to factory.js (refreshes the input-text when that tiddler changes, sets it to whatever is within the editTiddler)

  • a full-featured keyboard-selectable tags-dropdown

  • up/down selects a dropdown tag

  • enter adds the selected tag

  • escape clears the input field, or - if empty - cancels editing the tiddler

this adds a `saveTiddler` attribute to factory.js

if it's present it's used as a tiddler title where all the inputs/changes made by the user get stored
this can be used for selecting dropdown-entries that are filterable by an input field
the entries should be sorted by the `saveTiddler` and the original tiddler (`<$edit-text tiddler="originaltiddler" .../>`) can be updated with the selected dropdown-entry which will be shown within the input, without changing the filtering of the dropdown-items
@Jermolene
Copy link
Member

Hi @BurningTreeC this is intriguing, but I'm not sure I fully understand. Can you post a more complete example?

@BurningTreeC BurningTreeC changed the title inputs: second tiddler where edits can be stored [UI] add selectable tags-dropdown Nov 21, 2018
@BurningTreeC
Copy link
Contributor Author

@Jermolene - I've updated the first comment with the additions made

@BurningTreeC
Copy link
Contributor Author

we could refactor the logic for dropdown-selection into a macro, so that we can reuse it for the search-dropdown, fieldname-dropdown etc

@pmario
Copy link
Member

pmario commented Nov 22, 2018

do you have a minimal-test-case?

@BurningTreeC
Copy link
Contributor Author

@pmario - this is a current prerelease with my additions from this PR:

http://selectabledropdowninput.tiddlyspot.com

  • keep the top 3 state tiddlers open
  • try the tags Input of HelloThere
  • the first state is the real text value of the tags input
  • the second state is the user input by which the dropdown is filtered
  • the third state is the state used to refresh the input field (its text is updated and focus is set)
  • in the tags input try:
    -- up arrow / down arrow to select a tag from the dropdown
    -- write something to filter the dropdown
    -- up / down again, to see that the input updates with the selected tag but the dropdown filtering remains the same
    -- press Enter to add a tag (dropdown filtering remains the same), one can add more tags by same filter
    -- press Escape to reset the input field (all tags will be shown again, input gets cleared)
    -- on Escape: if all tags are shown and input field is clear, it cancels editing
    -- that's all

@pmario
Copy link
Member

pmario commented Nov 22, 2018

Great stuff! .... As you wrote, it would be cool to have it for the search input. Users would love this!

@BurningTreeC
Copy link
Contributor Author

BurningTreeC commented Nov 22, 2018

thanks @pmario , for the tags and search dropdowns, we have to take into account that we can add tabs within the dropdowns with different filterings

the select-logic by up/down needs the filters that are used within the current open dropdown tab

I think therefor we should put the filters (the tags and search dropdowns have two different filters for non-system and system) for the dropdown lists into a field somewhere. Then I can make the macro look up for the filters in those fields

BurningTreeC added 6 commits November 22, 2018 21:13
refactored the dropdown-selector logic into a `selectable-dropdown-input` macro
lets us define the state for a tabs-list explicitely, useful for selectable-dropdown-inputs, knowing the state tiddler from outside
\define tabs(tabsList,default,state:"$:/state/tab",class,template,buttonTemplate,retain)
\define tabs(tabsList,default,state:"$:/state/tab",class,template,buttonTemplate,retain,explicitState)
<$set name="qualifiedState" value=<<qualify "$state$">>>
<$set name="tabsState" filter="[<__explicitState__>minlength[1]] ~[<qualifiedState>]">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a workaround so that I know the state tiddler
the tabs macro simply uses a title without qualifying when explicitState is present

but a way to access a qualified state from outside the scope would be more elegant

<$keyboard key="((input-down))" actions="""<$macrocall $name="select-dropdown-actions" beforeafter="after" reverse="" editTiddler=<<__editTiddler__>> saveTiddler=<<__saveTiddler__>> refreshTiddler=<<__refreshTiddler__>> filter1={{{ [<__filterTiddler__>get[dropdown-filter-1]] }}} filter2={{{ [<__filterTiddler__>get[dropdown-filter-2]] }}} filterMinLength="$filterMinLength$"/>""">
<$keyboard key="((input-up))" actions="""<$macrocall $name="select-dropdown-actions" beforeafter="before" reverse="reverse[]" editTiddler=<<__editTiddler__>> saveTiddler=<<__saveTiddler__>> refreshTiddler=<<__refreshTiddler__>> filter1={{{ [<__filterTiddler__>get[dropdown-filter-1]] }}} filter2={{{ [<__filterTiddler__>get[dropdown-filter-2]] }}} filterMinLength="$filterMinLength$"/>""">
<$keyboard key="((input-accept))" actions="""<$macrocall $name="accept-input-actions" editTiddler=<<__editTiddler__>> acceptActions={{{ [<__filterTiddler__>get[accept-actions]] }}}/>""">
<$keyboard key="((input-clear))" actions="""<$macrocall $name="clear-input-actions" editTiddler=<<__editTiddler__>> saveTiddler=<<__saveTiddler__>> refreshTiddler=<<__refreshTiddler__>> clearEmptyActions={{{ [<__filterTiddler__>get[clear-input-empty-actions]] }}} refreshQualifier=<<__refreshQualifier__>>/>""">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would make sense to add some keyboard widgets here that can be configured with more actions without having to edit this tiddler

<$action-setfield $tiddler=<<__refreshTiddler__>> text=<<__refreshQualifier__>>/>
<$action-deletetiddler $tiddler=<<__editTiddler__>>/>
<$action-deletetiddler $tiddler=<<__saveTiddler__>>/>
<$action-setfield $tiddler=<<__refreshTiddler__>> text=""/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this sequence seems strange but is needed somehow like that if we don't come up with a different refresh mechanism

<$action-setfield $tiddler=<<__editTiddler__>> text=<<nextTag>>/>
<$action-deletetiddler $tiddler=<<__refreshTiddler__>>/>
<$action-setfield $tiddler=<<__refreshTiddler__>> refresh-qualifier={{{ [[$(tv-refresh-input-qualifier)$]] }}}/>
<$action-setfield $tiddler=<<__refreshTiddler__>> text=<<nextTag>>/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

theses two lines make the input field refresh after its text has changed through up/down-arrow selections

@BurningTreeC
Copy link
Contributor Author

Hi @Jermolene , are you interested in this?

You proposed a refresh-mechanism for inputs that restores focus. Have you got an idea how to do that?

@Jermolene
Copy link
Member

are you interested in this?

Very interested in gaining the functionality, but keen to do it right...

You proposed a refresh-mechanism for inputs that restores focus. Have you got an idea how to do that?

Tentatively, one possible approach is to have a tiddler called $:/state/current-focus that contains a string identifying the widget that owns the focus. Then each widget instance that wants to opt-in to the mechanism would need to have an attribute that gave the qualified ID for that widget. Then at refresh, the widget would detect whether it owned the focus, and set the focus to itself if so.

@Jermolene
Copy link
Member

btw the other thing that we should be able to fix in this way is maintaining the focus/selection while editing fields within the current tiddler...

@BurningTreeC
Copy link
Contributor Author

btw the other thing that we should be able to fix in this way is maintaining the focus/selection while editing fields within the current tiddler...

Ah yes, that should be done while we're on it.

So, if I understand it right, the widgets should define a unique ID and populate $:/state/current-focus with it when they get focus. When they leave focus, they check if the ID in that tiddler matches their ID and maybe set this.widget.shouldFocusAgain to true (?) and after a refresh of the widget, if this.shouldFocusAgain is true, it sets focus ...?

@Jermolene
Copy link
Member

Hi @BurningTreeC

That's the idea, yes. There are quite a few points to think about:

  • Triggering a refresh cycle every time the focus changes might be burdensome, so we may need to e.g. suppress change events for changes to the selection tiddler that have been immediately actioned
  • We'll also need to track the start and end positions of the text selection
  • I don't think that there is a reasonable basis for the widgets to generate the IDs themselves, but perhaps we should explore whether there are any viable strategies.

@BurningTreeC
Copy link
Contributor Author

BurningTreeC commented Jan 30, 2019

Triggering a refresh cycle every time the focus changes might be burdensome, so we may need to e.g. suppress change events for changes to the selection tiddler that have been immediately actioned

That's the reason why I originally made the refreshTiddler attribute, so it does this kind of refresh only if that tiddler refreshes. That tiddler should refresh with the user-action, so it needs to be updated with an action-widget after a user-interaction. I understand that it's a bit complicated.

We'll also need to track the start and end positions of the text selection

Yes, that's important. I was thinking about an inputManager startup module that initializes an array where inputs store their selectionStart and selectionEnd identified by the unique ID we're going to use ...

I don't think that there is a reasonable basis for the widgets to generate the IDs themselves, but perhaps we should explore whether there are any viable strategies.

I couldn't find another way ... a tiddler can have many inputs that edit the same tiddler and the same field/index and they could all be siblings, so a state qualifier using a combination of the editTitle, editField/Index and stateQualifier wouldn't be unique. I came up with adding the index of the widget within the array of its siblings to that identifier which is working well:

this.editQualifiedID = this.editTitle + (this.editField ? this.editField : "") + (this.editIndex ? this.editIndex : "") + this.getStateQualifier() + this.getOwnSiblingsWidgetIndex();

But possibly I'm just not seeing the best approach

@BurningTreeC
Copy link
Contributor Author

I see now that my approach would need to register each widget within an array of its parent widget

@pmario
Copy link
Member

pmario commented Jan 31, 2019

btw the other thing that we should be able to fix in this way is maintaining the focus/selection while editing fields within the current tiddler...

I think this would be a "killer" ;)

@BurningTreeC
Copy link
Contributor Author

Hi @Jermolene , this PR has merge conflicts that could be resolved. Do you have an idea/a roadmap how to go on with this? The unique widget id seems to be difficult to solve, this mechanism here uses a different approach, demanding that a given condition must be true in order for the input to refresh its text and focus again.

the condition is that a given refreshTiddler has changed and that the refreshCondition attribute is "true". it's a bit tricky as a mechanism, but it works

@Jermolene
Copy link
Member

Do you have an idea/a roadmap how to go on with this? The unique widget id seems to be difficult to solve, this mechanism here uses a different approach, demanding that a given condition must be true in order for the input to refresh its text and focus again.

It's tricky. The problem is that if we merge this then we're committed to supporting this technique forever, and that might limit our ability to address related problems in the future. On balance, experimental stuff like is good to chew over as a plugin. Do you think that's feasible, if we added enough core hooks?

@BurningTreeC
Copy link
Contributor Author

@Jermolene , I know I'm absolutely not convinced that this should be the mechanism to stick with forvever. but it's a mechanism that works, now.

the plugin proposal would be just right. about the hooks ... would you just add a hook in the factory.js refresh method, passing the widget?

@Jermolene
Copy link
Member

would you just add a hook in the factory.js refresh method, passing the widget?

Yes I think that's reasonable.

@BurningTreeC
Copy link
Contributor Author

would you just add a hook in the factory.js refresh method, passing the widget?

Yes I think that's reasonable.

ok, there are more points where factory.js would need to be extended, see https://github.com/Jermolene/TiddlyWiki5/pull/3565/files#diff-2788afb61c207f85b558b296f643ce5e

what's the best way to do it?

@Jermolene
Copy link
Member

what's the best way to do it?

Too many hooks at random places will also box us in too far. It may be reasonable to look at subclassing the existing factory.js, I'm not sure how much hackery would be needed to make that possible though...

@BurningTreeC
Copy link
Contributor Author

what's the best way to do it?

Too many hooks at random places will also box us in too far. It may be reasonable to look at subclassing the existing factory.js, I'm not sure how much hackery would be needed to make that possible though...

ok, I'll make a minimal working concept and see where subclasses make sense without overdoing it

@BurningTreeC
Copy link
Contributor Author

Hi @Jermolene,

in order to make this functionality work using hooks and a startup module, I need 3 hooks, one in the refresh method, one in the execute method and one in the getEditInfo method of factory.js

it's really simple to extend it this way, I think it'd be a great hackability extension

@BurningTreeC
Copy link
Contributor Author

in order to make this functionality work using hooks and a startup module, I need 3 hooks, one in the refresh method, one in the execute method and one in the getEditInfo method of factory.js

nah, this was exaggerated, I only need 1 hook in the refresh method

@BurningTreeC BurningTreeC deleted the patch-48 branch June 23, 2019 13:29
@BurningTreeC BurningTreeC restored the patch-48 branch July 4, 2019 07:33
@BurningTreeC
Copy link
Contributor Author

opening this again for discussion - I got a version working that uses the new widget-subclassing here: http://selectable-inputs.tiddlyspot.com/

@BurningTreeC BurningTreeC reopened this Jul 4, 2019
@BurningTreeC
Copy link
Contributor Author

closing in favor of a new attempt

@BurningTreeC BurningTreeC deleted the patch-48 branch July 20, 2020 06:33
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.

3 participants