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

Update styles for better actions alignment support in search (related to #61532) #63965

Conversation

IllusionMH
Copy link
Contributor

@IllusionMH IllusionMH commented Nov 29, 2018

This in second iteration of #63457 for #61532.
New property logic that controls class responsible for aligment was restored in 1c62ed7.

This PR updates styles and layout for results rows and actions bar to achieve next goals:

  • Count badge is always aligned to the left (at the end of content) - this is static info that is related to content (file name or folder name) and it makes sense to treat it in the same way as line numbers in match strings.
  • Count badge is always visible - it doesn't make sense to hide count badge if actions are on the right side or search is wide enough to display both badge and actions. It will be shifted if search panel is narrow (same behavior as with line numbers)
  • Unified layout and styles for match lines and file/folder lines - added wrapper that allows easy achieve left and right action alignment and avoid Search: Can't select search results #63834.
  • Margins for badge now are same as for actions, to achieve similar alignment
    (Should I also update margins for line numbers, because now scroll bar can overlap line numbers, but not actions/count badge? Or I can create separate issue for it.)
Gif (34 MB)

Link to gif (expires in 60 days)

(GitHub doesn't allow to upload it and doesn't play animation when it is embedded)

@roblourens
Copy link
Member

Count badge is always aligned to the left (at the end of content) - this is static info that is related to content (file name or folder name) and it makes sense to treat it in the same way as line numbers in match strings.

I disagree, just because I think it looks better in a narrow view, aligned to the right. It's so visually heavy that I don't like the way it looks when not regularly lined up on the right. I actually made this change once and got complaints.

It will be shifted if search panel is narrow (same behavior as with line numbers)

Not sure about this one, I'm not sure I like the "shift" of a heavy UI element on every mouse hover.

(Should I also update margins for line numbers, because now scroll bar can overlap line numbers, but not actions/count badge? Or I can create separate issue for it.)

I don't think that's a problem... it can overlap the result text too. I'd rather use those extra 10 pixels for content.

@IllusionMH
Copy link
Contributor Author

I see your point about heavy element and agree that on narrow layout it might look distracting.

What do you think about always showing badge on wide layouts and if it should depend on actionsPosition value? Would you consider this or PR should be closed?

It might still might be shifting for really long paths, but I wasn't able to get to that state in this repo (not searched in node_modules, but really depends on screen).

@roblourens
Copy link
Member

It might still might be shifting for really long paths

It will always shift when they are on the right. I still just don't like shifting things around, whether it's in a narrow or wide view. It looks like something is going wrong with the UI.

@IllusionMH
Copy link
Contributor Author

IllusionMH commented Dec 1, 2018

I mean:

  • in narrow (<1000px) view badge will be aligned to the right and replaced with action on hover (no changes from current behavior)

  • in wide view - badge aligned to the end of content (as it is now with search.actionsPosition: auto)

    • for auto badge stays visible on hover and action will appear after the badge (instead of replacing badge)
    • for right badge stays visible on hover and action will appear at the right edge of view.

So for wide view to see any shift - file name and path combined should be at least ~875px.

As like line numbers are near matched content, this will keep informational badge close to file name/path (there are no need to align count in one line as with actions that are clickable).

@roblourens
Copy link
Member

I see. Yeah I guess I would take that.

@IllusionMH
Copy link
Contributor Author

Closing in favor of #64578

@IllusionMH IllusionMH closed this Dec 7, 2018
@IllusionMH IllusionMH deleted the updated-search-actions-align-option-61532 branch December 10, 2018 23:41
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants