Skip to content

Update sidebar styles#4052

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

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

Conversation

@daviddossett

@daviddossett daviddossett commented Oct 13, 2022

Copy link
Copy Markdown
Contributor
  • 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
Comment thread webviews/editorWebview/index.css
Thomas1664
Thomas1664 reviewed Oct 14, 2022
View reviewed changes
Comment thread webviews/editorWebview/index.css
@Thomas1664

Thomas1664 commented Oct 14, 2022

Copy link
Copy Markdown
Contributor

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

Before:

image

After:

image

Thomas1664
Thomas1664 reviewed Oct 14, 2022
View reviewed changes
Comment thread webviews/editorWebview/index.css Outdated
Comment thread webviews/components/sidebar.tsx
@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

Thomas1664 commented Oct 14, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor
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

daviddossett commented Oct 14, 2022

Copy link
Copy Markdown
Contributor Author

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