Skip to content

picker: Prevent clicking non-selectable entries from confirming selection#50705

Merged
MrSubidubi merged 4 commits intozed-industries:mainfrom
Dnreikronos:fix/picker-header-click-confirms-item
Mar 5, 2026
Merged

picker: Prevent clicking non-selectable entries from confirming selection#50705
MrSubidubi merged 4 commits intozed-industries:mainfrom
Dnreikronos:fix/picker-header-click-confirms-item

Conversation

@Dnreikronos
Copy link
Copy Markdown
Contributor

Closes #50627

Before you mark this PR as ready for review, make sure that you have:

  • Added a solid test coverage and/or screenshots from doing manual testing
  • Done a self-review taking into account security and performance aspects
  • Aligned any UI changes with the UI checklist

Videos:

Before:
https://drive.google.com/file/d/1PahGhfx-wq9cyNqqMlctb9Np7M1Meo71/view?usp=sharing
After:
https://drive.google.com/file/d/135W6MQ9hKBurw5Z7YpHQQhoQwkh1NvUb/view?usp=sharing

Release Notes:

  • Fixed clicking on non-selectable picker entries (e.g. section headers) confirming the currently selected item.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 4, 2026
@maxdeviant maxdeviant changed the title picker: Prevent clicking non-selectable entries from confirming selec… picker: Prevent clicking non-selectable entries from confirming selection Mar 4, 2026
MrSubidubi

This comment was marked as outdated.

Copy link
Copy Markdown
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

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

Thanks, this looks like a good fix for the issue at hand.

I'd like if we could add a test for this behavior to ensure that this does not regress. Could you look into that?

Also, one thing that popped up while I looked at this is that we actually show a pointer cursor when hovering the element despite it not being clickable. Could I interest you into looking to fix that as well perhaps? Not insisting, just curious.

@MrSubidubi MrSubidubi self-assigned this Mar 5, 2026
@Dnreikronos
Copy link
Copy Markdown
Contributor Author

Thanks, this looks like a good fix for the issue at hand.

I'd like if we could add a test for this behavior to ensure that this does not regress. Could you look into that?

Also, one thing that popped up while I looked at this is that we actually show a pointer cursor when hovering the element despite it not being clickable. Could I interest you into looking to fix that as well perhaps? Not insisting, just curious.

Of course, I will look to that now!

@Dnreikronos
Copy link
Copy Markdown
Contributor Author

Thanks, this looks like a good fix for the issue at hand.

I'd like if we could add a test for this behavior to ensure that this does not regress. Could you look into that?

Also, one thing that popped up while I looked at this is that we actually show a pointer cursor when hovering the element despite it not being clickable. Could I interest you into looking to fix that as well perhaps? Not insisting, just curious.

@MrSubidubi, changes implemented as you requested.

Follow bellow a showcase video testing the cursor pointer on the hover:
Kooha-2026-03-05-09-00-30.webm

@Dnreikronos Dnreikronos requested a review from MrSubidubi March 5, 2026 12:04
Copy link
Copy Markdown
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

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

Very very nice, thanks for looking into this and the quick follow-up!

Two minor comments and we can 🚢

@Dnreikronos Dnreikronos requested a review from MrSubidubi March 5, 2026 14:42
Copy link
Copy Markdown
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

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

Awesome work here and very much appreciate the work that went into the test. Thank you!

@MrSubidubi MrSubidubi enabled auto-merge (squash) March 5, 2026 18:37
@Dnreikronos
Copy link
Copy Markdown
Contributor Author

Awesome work here and very much appreciate the work that went into the test. Thank you!

Appreciate your time invested, @MrSubidubi!
Looking to next issues where we can work together!

auto-merge was automatically disabled March 5, 2026 18:46

Head branch was pushed to by a user without write access

@Dnreikronos
Copy link
Copy Markdown
Contributor Author

@MrSubidubi , can you trigger the CI pipeline again?
I fixed the rustfmt formatter pipeline error.

@MrSubidubi MrSubidubi enabled auto-merge (squash) March 5, 2026 18:51
@MrSubidubi MrSubidubi merged commit 94154aa into zed-industries:main Mar 5, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Recent Projects" picker title is weird

2 participants