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

Delete key, focus/blur and dropdown improvements for select mode with userInput #1410

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Massi-X
Copy link
Contributor

@Massi-X Massi-X commented Nov 14, 2024

1: When you Tab into the input and then immediately click backspace the focus was lost. This PR fixes the behavior by only removing tags after blur, so that we can still write into the input.

2: Previously the focus class remained even after leaving the input, fixed by adding a "Tab" handler for select mode too.

3: Previously the dropdown stayed open when navigating with tab key (related to point 2), fixed by adding a "Tab" handler for select mode too.

4: The input focus was inconsistent on suggestion selection with a tag present or not, fixed by regaining focus to the input every time a suggestion is clicked.

5: Repopulate suggestions dropdown after selection (currently the dropdown is stuck to a partial state, which is not very desirable in select mode).

6: Do not highlight next option when in select mode (this is not desired, the current one is still present unlike the other modes).

When using delete key the behavior was inconsistent and also led to focus lost. Fix this by never calling removeTags() while we are editing but only after blur
Previously the focus class remained even after leaving the input, moreover the dropdown stayed open if navigating with tab. At last the input focus was inconsistent on suggestion selection with a tag present or not, now every time a suggestion is clicked, the input regain focus.
@Massi-X Massi-X changed the title Delete key select Delete key, focus/blur and dropdown improvements for select mode with userInput Nov 14, 2024
Currently after the user inputs some text and then select a suggestion, the dropdown is stuck to that suggestion list. To see all the suggestions again the dropdown must be toggle. Fixed by showing all suggestions after selection only for select mode (should this be considered for non-select too?)
Do not higlight the next option while in select mode but keep the current one
@yairEO
Copy link
Owner

yairEO commented Nov 23, 2024

I need videos showcasing the things these changes are trying to solve.. I do not understand from the description what exactly is the situation for each of your changes and I cannot blindly merge such sensitive changes without completely understanding the problems first and am able to reproduce myself

I am a Windows user and this is my favorite video capturing software:
https://www.screentogif.com

@Massi-X
Copy link
Contributor Author

Massi-X commented Nov 24, 2024

I'm going to first push some commits because of the changes in 4.32.1 and also 'cause I find better ways to do things... I will update you with videos too.

@Massi-X Massi-X force-pushed the delete-key-select branch 3 times, most recently from d9926e0 to 2b66604 Compare November 25, 2024 22:46
@Massi-X
Copy link
Contributor Author

Massi-X commented Nov 25, 2024

OK, I have videos for all the parts of this commit and I'm going to step through that one at a time. First some notes: I have updated to 4.32.1 and reapplied the patches one by one to confirm the issues are still present. In the videos attached I'm using my implementation because it was easier for me, but I tried everything on jsbin with a "regular" install and it is the same.

  1. Tab into input and click backspace, the input loses focus. Fixed with commit 2b66604 (old and superseded: 542e73e)
1.mov
  1. Not applicable anymore in 4.32.1 but it was a consequence of other commits, nothing to revert

  2. Tab into the input so the dropdown opens, now click a suggestion with the mouse. If you now try to click outside the input the dropdown won't close until you focus it again, if you instead try to "tab" out the focus went back to the top of the page (because the focus after the selection was on the dropdown and not on the input). Fixed with commit 2c8ccab and e6381e3

3.mp4
  1. For the video look at 3. I try to explain this one better: When you select a suggestion, the input didn't regain focus most of the time. The input should always get the focus back after a selection, this was already in the code (DOM.input.focus()) but didn't work because in select mode the element to focus is .tagify__tag-text

  2. If you start to input some text you will end up with a filtered suggestion list; that's fine but after you select something the list is still filtered. I understand this is correct behavior for the other modes, but for select is not desirable. To bring back the full list it's necessary to close and reopen the dropdown. b35a43e fixes this by showing all suggestion after selection

5.mov
  1. 16e6788 Isn't really an issue cause right now the highlight goes back to the first option anyway so I can't show this in a video. It was for future proofing (I'm working on keeping the highlight in the same spot and some other things with another PR EDIT: this is Keep highlight in select mode #1420)

As a bonus: I now found where the commit I was talking in #1408 (comment) belongs to, and it is here! As a consequence of e6381e3 the dropdown does not close in every case, and the global handler onClickAnywhere fails because the search for target.closest(".tagify__dropdown") returns null. I moved that commit here 5ae3c62 Superseded by d9d9ece

The dropdown was incorrectly closing with right arrow too
@yairEO
Copy link
Owner

yairEO commented Dec 7, 2024

Hi, I cannot reproduce much of your videos because I don't know your exact Tagify configurations, so I cannot personally confirm the issues. Can you please share it here?

@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 7, 2024

Sorry I'm not on my PC, it should be about like this. I used a stripped down version with less settings but I don't have it on hand right now. Tell me if this is enough (I tried on jsbin and all the issues were present)

tagify = new Tagify(elem), {
   enforceWhitelist: true,
   whitelist: whitelist,
   dropdown: {
	enabled: 0,
	closeOnSelect: false,
	mapValueTo: 'name',
	searchKeys: ['name', 'value'],
	includeSelectedTags: true,
	maxItems: Infinity
   },
   mode: "select",
   tagTextProp: 'name',
   originalInputValueFormat: valuesArr => valuesArr.map(item => item.value)
});

UPDATE with real code

@@ -96,7 +96,11 @@ export default {
this.state.autoCompleteData = selectedElmData;
this.input.autocomplete.set.call(this, value)
return false
} else if( e.key == 'Tab' && !_s.autoComplete.tabKey && isSelectMode && _s.userInput ) { // do not forget to hide the dropdown in select mode if Tab is pressed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1414 depends on this

@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 8, 2024

So it seems like you fixed #1408 and 5ae3c62 in a similar fashion with d9d9ece
I'm going to revert that commit

When input is in edit mode the checkmark stayed on the previous item
@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 9, 2024

Commit 41178f7

I found another issue... Added here (see video). I have tested on jsbin too the only difference here is that because the dropdown closes you can't see the bug (the dropdown closing is itself a bug, given the option to not close it is present).

When you click on the input and you are in edit mode (caret blinking), if you select a suggestion with the mouse the checkmark won't be updated (in the video only on second click because I exited edit mode).

1.mov

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 needed to prevent the dropdown from closing even when closeOnSelect is false

@yairEO
Copy link
Owner

yairEO commented Dec 10, 2024

Sorry I am too slow to respond to your well-done detective work!
Never did I have any contributor as eager as you are.

I work a very demanding full-time job (as a frontend developer) and have so little time for other things. I also have much more (personal) open source and non-open-source projects to maintain...

So mostly the only time I have is a little over the weekend

@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 10, 2024

The same for me 🤣 except that I do my things at night 🌉

I suspected you were only available on the weekends but no worry I'm not here to rush you. I've implemented the changes in production on my module so we can also have a reality check (small as it is...)

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.

2 participants