[web-console] Improve empty-state and Enter submit for tags#6621
Conversation
Submit the create tag form on Enter, skip the empty search list when no tags exist, and open create directly in that case. Fixes feldera#6596 Fixes feldera#6595 Signed-off-by: mrswastik-robot <swastikpatel29@gmail.com>
mythical-fred
left a comment
There was a problem hiding this comment.
Nice tight fix for both bugs — the <form onsubmit> pattern is the right way to handle Enter, and opening create directly when knownTags.size === 0 is exactly the UX the issue asked for.
Blocker: missing tests.
This changes behavior in two user-visible ways and web-console has a hard rule (from Gerd) that behavioral changes need tests. Concrete suggestions using Vitest + @testing-library/svelte:
- Enter submits the create form — render the create panel, type a tag name,
userEvent.keyboard('{Enter}'), assertonSubmitwas called (or the tag appears in the mock store). - Enter is a no-op when
submitDisabledis true — same setup with the button in its disabled state (e.g. empty/duplicate name), assertonSubmitwas not called. - Empty pool skips the search list and opens create directly — render
TagswithknownTags = new Set(), click the trigger, assert the create form is visible and the search<input type="search">is not in the document. - Non-empty pool still shows search — render with a couple of tags, click trigger, assert the search input is present.
The reviewer bandwidth is not the issue here — it's that the web-console has zero test coverage and every PR that changes behavior without adding tests locks that in. Please add a Vitest spec next to Tags.svelte.
Minor nit (non-blocking): in the open closure, when knownTags.size === 0 you call openCreate('') and then toggle(). That's fine as long as openCreate doesn't itself toggle — worth a quick check that opening the popup once (not twice) is what happens.
|
hey @Karakatiza666, should I include tests too? |
|
Yes, please. Add unit test (see *.svelte.spec.ts) to check that submitting through Ener works, selecting a color through Tab + Spacebar works (it does now), and that when clicking on "+ tag" when no tags exist yet the "Create tag" form pops up right away |
Karakatiza666
left a comment
There was a problem hiding this comment.
The fix for both issues is minimal, idiomatic, and works as expected. Thank you for the contribution!
Add tests as I mentioned above and you can merge.
|
Well, we can merge |
should I go for the tests? or is it alright? |
|
He said he would like tests. |
ahh! ok got it 😂, my bad! (should have put an asterisk there btw 🤐) |
|
hey @mihaibudiu and @Karakatiza666. While writing the |
|
Right. Yes, I'd consider that expected behavior. So in the test, after selecting the color, press Shift + Tab the same amount of times to navigate the focus back to the input field |
Signed-off-by: mrswastik-robot <swastikpatel29@gmail.com>
Added the test, lmk if any change is required :) |
|
Looking good! |
Submit the create tag form on Enter, skip the empty search list when no tags exist, and open create directly in that case.
Fixes #6595
Fixes #6596
Describe Manual Test Plan
:5173against pipeline-manager with--dev-modeon:8080.Checklist
Breaking Changes?
None.
Describe Incompatible Changes
None.