Skip to content

Update sidebar styles#4052

Merged
daviddossett merged 6 commits intomainfrom
ddossett/sidebar-updates
Oct 14, 2022
Merged

Update sidebar styles#4052
daviddossett merged 6 commits intomainfrom
ddossett/sidebar-updates

Conversation

@daviddossett
Copy link
Contributor

@daviddossett daviddossett commented Oct 13, 2022

  • Updates sidebar styles to follow designs in Polish Extension UI #4029
  • Fixes icon button hover and focus styles to match core
  • Swaps add icons for settingsGear icons
  • Styles labels to look act reflow like github.com
  • Adjusts app grid layout

Detail

195714899-ddc6ea9c-34f8-4f59-8f53-97c42a70650c

Before / After

CleanShot 2022-10-13 at 14 34 57@2x

Demo

CleanShot.2022-10-13.at.14.26.17.mp4

Prior art

CleanShot 2022-10-13 at 14 36 20@2x

cc @digitarald @esonnino

roblourens
roblourens previously approved these changes Oct 14, 2022
View reviewed changes
alexdima
alexdima previously approved these changes Oct 14, 2022
View reviewed changes
wusar
wusar previously approved these changes Oct 14, 2022
View reviewed changes
Thomas1664
Thomas1664 reviewed Oct 14, 2022
View reviewed changes
Thomas1664
Thomas1664 reviewed Oct 14, 2022
View reviewed changes
@Thomas1664
Copy link
Contributor

Thomas1664 commented Oct 14, 2022

There is also less space between icon and text. I.e.:

Before:

image

After:

image

Thomas1664
Thomas1664 reviewed Oct 14, 2022
View reviewed changes
@daviddossett daviddossett dismissed stale reviews from wusar and alexdima via b4abf1b October 14, 2022 18:14
@daviddossett daviddossett changed the title Update sidebar styles and layout Update sidebar styles Oct 14, 2022
@daviddossett daviddossett merged commit ea4971f into main Oct 14, 2022
@daviddossett daviddossett deleted the ddossett/sidebar-updates branch October 14, 2022 22:06
@Thomas1664
Copy link
Contributor

Thomas1664 commented Oct 14, 2022 •
edited
Loading

CleanShot 2022-10-14 at 11 10 35@2x

This works for me—I specifically added hover and focus behavior for these buttons.

(Although you remove them in another PR) it does not work for the header icons

@Thomas1664 Thomas1664 mentioned this pull request Oct 14, 2022
Closed
@daviddossett
Copy link
Contributor Author

daviddossett commented Oct 14, 2022

Ah ok, right—that issue should go away once #4050 goes in. After that, I'll formally replace all icon buttons with the webview UI toolkit counterparts.

This was referenced Oct 14, 2022
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roblourens roblourens roblourens approved these changes

@alexdima alexdima alexdima left review comments

+2 more reviewers

@Thomas1664 Thomas1664 Thomas1664 left review comments

@wusar wusar wusar left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@daviddossett daviddossett

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@daviddossett @Thomas1664 @roblourens @alexdima @wusar