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

Option to control actions alignment in search results #63457

Merged
merged 2 commits into from Nov 27, 2018

Conversation

Projects
None yet
4 participants
@IllusionMH
Copy link
Contributor

IllusionMH commented Nov 19, 2018

Fixes #61532

This PR adds new search.actionsPosition option that can force search result action (replace, replace all, dismiss) to be always displayed at the right edge of the panel.

Other change - count badge for files and folders matches are displayed at the end of match and visible on hover. Only negative aspect of later change is shift of badge in really narrow search block when actions become visible on hover. Since count badge doesn't have click handlers its shift shouldn't be so bad (in comparison with any actionable element).

@roblourens looks like I've removed changes introduces in your commit. Could you please describe what issues this commit fixed or give me a link to the issue. I will check that this PR doesn't regress there.

Also I haven't found test related to search results badge/actions layout etc. Are there tests that I should update in this PR?

@roblourens
Copy link
Member

roblourens left a comment

Looks great, thanks for the PR! And for cleaning up the CSS 👍

@roblourens roblourens merged commit e8db92f into Microsoft:master Nov 27, 2018

2 checks passed

VS Code #20181119.58 succeeded
Details
license/cla All CLA requirements met.
Details

@roblourens roblourens added this to the November 2018 milestone Nov 27, 2018

'search.actionsPosition': {
type: 'string',
enum: ['auto', 'right'],
default: 'auto',

This comment has been minimized.

@capaj

capaj Nov 27, 2018

why is this not right?
that would make a ton more sense than auto

This comment has been minimized.

@IllusionMH

IllusionMH Nov 27, 2018

Contributor

While I personally prefer right I decided to preserve behavior that was default in 1.29 because my issue (filled even before 1.29 release) got 0 👍 and only two supporting comments.

In any way it may be discussed and changed if maintainers are OK with it.

This comment has been minimized.

@capaj

capaj Nov 27, 2018

I've already opened a PR to be address this. Thanks for making this configurable! It's been plaguing me last few days.

This comment has been minimized.

@roblourens

roblourens Nov 27, 2018

Member

We've discussed this a bunch already, I'm not going to change the default because 'right' doesn't make sense to me for wide views on large monitors. The setting is here for your to customize however you want.

@joaomoreno

This comment has been minimized.

Copy link
Member

joaomoreno commented Nov 27, 2018

This caused #63834 so I will revert it for now.

joaomoreno added a commit that referenced this pull request Nov 27, 2018

Revert regression in search viewlet
Fixes #63834

Revert "Cleanup new setting description"

This reverts commit b995518.

Revert "Merge pull request #63457 from IllusionMH/search-actions-align-option-61532"

This reverts commit e8db92f, reversing
changes made to 4201f51.
@IllusionMH

This comment has been minimized.

Copy link
Contributor

IllusionMH commented Nov 27, 2018

@joaomoreno thank you for notification.
I was able to reproduce it "search.actionsPosition": "right" or "search.actionsPosition": "auto" and view is "narrow" (less than 1000px wide).
Are there other cases that I should check?

I will submit new PR today.

roblourens added a commit that referenced this pull request Nov 27, 2018

@IllusionMH IllusionMH deleted the IllusionMH:search-actions-align-option-61532 branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment