Skip to content

feat: Form.TagPicker support#1158

Draft
bpavuk wants to merge 1 commit into
vicinaehq:mainfrom
bpavuk:bpavuk/form-tag-picker-support
Draft

feat: Form.TagPicker support#1158
bpavuk wants to merge 1 commit into
vicinaehq:mainfrom
bpavuk:bpavuk/form-tag-picker-support

Conversation

@bpavuk
Copy link
Copy Markdown
Contributor

@bpavuk bpavuk commented Mar 3, 2026

[ THIS IS INTENDED SOLELY FOR UI/UX REVIEW PURPOSES; PLEASE DON'T MERGE UNTIL DEBUG LOGGING IS REMOVED ]

closes #746, improves compatibility with Linear and Obsidian Raycast plugins.

a short overview:

  • flake.nix, .gitignore, CMakeLists.txt - that's to use Qt Creator and/or qmlls on NixOS
  • FormTagPicker.qml and TagPickerTag define UI
  • parsing is spread thin across the codebase, but the most important parts are in tag-list.cpp and form-model.cpp
  • fuzzy search implemented as a separate context property (is this how it's called?) because I can't think of a cleaner way to hand fuzzy search to C++

@bpavuk
Copy link
Copy Markdown
Contributor Author

bpavuk commented Mar 3, 2026

also, formatting in QML files is absolute hell. I did not take time to write .editorconfig for that

@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch from b5350c7 to 7104486 Compare March 3, 2026 18:12
@bpavuk bpavuk marked this pull request as draft March 3, 2026 18:14
@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch 3 times, most recently from 469c1e6 to bd0c258 Compare March 7, 2026 21:16
@aurelleb
Copy link
Copy Markdown
Contributor

aurelleb commented Mar 17, 2026

Ok so I finally got to testing this and overall it's really really good IMO.

Two main remarks:

  • I think icon and label should be spaced out a bit more
image
  • we need to have up/down working to navigate the list otherwise it's really misleading. IIRC you had some trouble implement this one, but it's definitely doable as we do in the "Create Shortcut" view:
image

Also I recommend rebasing on main as soon as you can to avoid further conflicts and take in updates.

aurelleb
aurelleb reviewed Mar 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@aurelleb aurelleb left a comment

Choose a reason for hiding this comment

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

Also, I didn't test this but feel free to edit the existing 'form' extension playground under ./extra/extension-boilerplate to test two more scenarios:

  • controlled tag picker (value is set on the react side, and state is changed when the change event is emitted)
  • correct handling of reactivity. If the react side sends an entire new tag list all of a sudden it should gracefully update.

Comment thread src/server/src/qml/qml/FormTagPicker.qml Outdated
}

Keys.onPressed: (event) => {
if (event.key === Qt.Key_Return || event.key === Qt.Key_Enter) {
Copy link
Copy Markdown
Contributor

@aurelleb aurelleb Mar 17, 2026

Choose a reason for hiding this comment

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

you should check modifier here (or lack of it using === Qt.NoModifier) so that it doesn't block shift+return or other stuff, as it's used to submit the form.

Copy link
Copy Markdown
Contributor Author

@bpavuk bpavuk Mar 31, 2026

Choose a reason for hiding this comment

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

it doesn't block shift+enter - the behavior is alright, I can submit the form while still picking a tag. are you sure I still need to check for modifier?

Comment thread src/server/src/qml/launcher-window.cpp Outdated
Comment thread src/server/src/qml/extension-form-model.cpp Outdated
Comment thread src/server/src/qml/extension-form-model.cpp Outdated
Comment thread src/server/src/qml/extension-form-model.cpp Outdated
Comment thread CMakeLists.txt
cmake_minimum_required(VERSION 3.16)
set(PROJECT_NAME vicinae)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
set(QT_QML_GENERATE_QMLLS_INI ON)
Copy link
Copy Markdown
Contributor

@aurelleb aurelleb Mar 17, 2026

Choose a reason for hiding this comment

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

qmlls support was merged on main some time ago, take that instead of doing this here

Copy link
Copy Markdown
Contributor Author

@bpavuk bpavuk Mar 31, 2026

Choose a reason for hiding this comment

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

hmm, can you tell where specifically? at the moment, fuzzy search indicates that this is the only instance where qmlls is mentioned at all. I can remove this option once the feature is done, but I really really need qmlls here - much computationally cheaper than rebuilding the entirety of Vicinae every time

Comment thread src/typescript/api/package-lock.json
@bpavuk
Copy link
Copy Markdown
Contributor Author

bpavuk commented Mar 17, 2026

Also I recommend rebasing on main as soon as you can to avoid further conflicts and take in updates.

I can deal with conflicts anytime. even if they accumulate, they are easy to resolve with jj

@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch from bd0c258 to 413c2eb Compare March 31, 2026 12:03
@bpavuk
Copy link
Copy Markdown
Contributor Author

bpavuk commented Mar 31, 2026

just rebased everything. took like 10 minutes - nothing too serious. looking into comments closer now...

@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch 3 times, most recently from b3351d9 to ca98f17 Compare March 31, 2026 12:49
bpavuk
bpavuk commented Mar 31, 2026
View reviewed changes
Comment thread src/server/src/qml/qml/AlertDialog.qml
onAboutToShow: {
_confirmed = false;
Qt.callLater(cancelBtn.forceActiveFocus);
}
Copy link
Copy Markdown
Contributor Author

@bpavuk bpavuk Mar 31, 2026

Choose a reason for hiding this comment

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

for the record - this is by make qmlformat

@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch from ca98f17 to 28ae3ed Compare March 31, 2026 16:36
@bpavuk
Copy link
Copy Markdown
Contributor Author

bpavuk commented Mar 31, 2026

we need to have up/down working to navigate the list otherwise it's really misleading.

I'd do a UX cleanup pass here in whole. will take on that tomorrow and until the end of the week. this time for real :)

@bpavuk bpavuk requested a review from aurelleb March 31, 2026 17:16
@bpavuk bpavuk force-pushed the bpavuk/form-tag-picker-support branch from 28ae3ed to a09c9c0 Compare April 2, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aurelleb aurelleb Awaiting requested review from aurelleb

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Form.TagPicker support

2 participants

@bpavuk @aurelleb