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

Clean up panel tabs and icons #51061

Merged
merged 17 commits into from
Jun 6, 2018
Merged

Clean up panel tabs and icons #51061

merged 17 commits into from
Jun 6, 2018

Conversation

miguelsolorio
Copy link
Contributor

@miguelsolorio miguelsolorio commented Jun 4, 2018

This PR cleans up the following items in the panel:

Tabs

  • Adjusts font-size and line-height so that are consistent with the tabs in settings
  • Adjusts margin on first item so that the spacing equals 20px
  • Reduces the spacing between tab titles so they equal 20px (instead of 36px)
  • Adjusts vertical padding so they align horizontally with right actions

Actions

  • Aligns input items vertically via flex-box instead of margins
  • Reduces the height of the filter input so it is vertically centered
  • Adjusts margin/padding so they are even with action icons

Icons

  • Adjusts icons so they fit on a 16x16 square
  • Makes the clear icons identical across tabs
  • Adjusts color of icons that are not consistent

Example showing adjustments:
image

Fixes #19052

/cc @bpasero

@bpasero
Copy link
Member

bpasero commented Jun 4, 2018

@misolori it looks like the close button is now larger, could that be? I think actions should be aligned to the editor toolbar (when tabs are disabled):

image

@miguelsolorio
Copy link
Contributor Author

@bpasero yes, I did slightly increase the close button from 8.717 px to 10px to closer align with the other icons that are 14px.

I'm not following on the actions being aligned to the toolbar, can you elaborate?

@bpasero
Copy link
Member

bpasero commented Jun 4, 2018

@misolori I just thought if we change the size of icons in a toolbar, the size should be changed in other places too, no? We use the close action also in the editor toolbar to close an editor and if tabs are disabled it will show up in a quite similar location as the panel actions.

@miguelsolorio
Copy link
Contributor Author

@bpasero ah yes, those should be consistent. I’ll update those other close buttons, thanks for catching that.

@Tyriar
Copy link
Member

Tyriar commented Jun 4, 2018

Looks great The only thing I notice is that overflow active/focus underlines aren't working sometimes in this build. I don't see this on insiders:

screen shot 2018-06-04 at 9 16 45 am

screen shot 2018-06-04 at 9 16 51 am

@Tyriar Tyriar added this to the June 2018 milestone Jun 4, 2018
@isidorn
Copy link
Contributor

isidorn commented Jun 4, 2018

@misolori thanks a lot for this great PR. Here's some feedback after trying it out:

  • Same as @Tyriar I noticed that the blue focus sometimes disapears - this is due to the fact that it is a couple of pixels below the title and now there is probable not enough space for it to be rendered.
  • when dragging a panel tab the tab image is very blurry compared to the insiders
  • Clear icon is not the same in the panel area and in the search and extensions. We should use the same icon everywhere especialy since the search can be moved to the panel and then it is particularly inconsistent. "search.location": "panel" if you want to do that
  • off topic: panel icons still feel too heavy and out of place to me (look at that giant plus). Though I think you have a differnt PR where you look into icons

Nice work!

@miguelsolorio
Copy link
Contributor Author

@bpasero I updated the rest of the close icons. I noticed that you had previously updated the icon color for the close-inverse.svg (from #C5C5C5 to #e8e8e8 ). I updated the color to be consistent with the rest of the toolbar icons (#c5c5c5 ). Let me know if you want to change that back.

@Tyriar I updated the height of the toggle more button that was causing the border to be cut off so it should work now:

@isidorn

  • See response above for border getting cut off
  • I'm not able to repro this blurriness, can you send a video example? What would cause this to be blurry?.
  • Updated the clear icon for the search + extension views
  • Completely agree with the heaviness of the icons, hoping to address this in another PR soon 😁

@miguelsolorio
Copy link
Contributor Author

I also updated the spacing on the tabs so that they're uniform when they're dragged (still equals 20px). Here's a before and after:
image

@isidorn
Copy link
Contributor

isidorn commented Jun 5, 2018

@misolori looks great and if others agree I will merge this in.

The only left issue is the blurry tab - here's a gif showing that. First is your branch - notice how it is not so crisp, second is stable. I do not find this to be a big issue, so we can merge even if you plan to fix this later.

paneldrag

@miguelsolorio
Copy link
Contributor Author

@isidorn thanks, I'll create an issue to track the blurry problem and make a fix later.

@isidorn isidorn merged commit d114692 into master Jun 6, 2018
@isidorn isidorn deleted the misolori/adjust-panel-alignment branch June 6, 2018 07:23
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 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.

Panel header spacing/alignment issues
4 participants