Skip to content

Update header UI#4050

Merged
daviddossett merged 9 commits into
mainfrom
ddossett/header-updates
Oct 14, 2022
Merged

Update header UI#4050
daviddossett merged 9 commits into
mainfrom
ddossett/header-updates

Conversation

@daviddossett

@daviddossett daviddossett commented Oct 13, 2022

Copy link
Copy Markdown
Contributor

This PR addresses the following:

  • Updates header styles and layout to match the design in Polish Extension UI #4029 and aligns with extension detail page in core
  • Uses extension detail- like buttons instead of mixed icons/large buttons
  • Ensures content reflows on small viewport/editor sizes
  • Updates max width of overview content
  • Refactors header JSX by extracting out Title, Subtitle, and ButtonGroup into their own components.

Before

CleanShot 2022-10-13 at 11 16 32@2x

After

CleanShot 2022-10-13 at 11 05 35@2x

Demo

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

@alexr00 I didn't have the time but I think we should replace the title editing form with a quick pick like how Settings Profile rename works. Thoughts?

cc @digitarald

rzhao271
rzhao271 previously approved these changes Oct 13, 2022
Comment thread webviews/components/header.tsx Outdated
Comment thread webviews/components/header.tsx
@Thomas1664

Copy link
Copy Markdown
Contributor

A visual distinction between drafts and open PRs like on the GitHub website would be nice, i.e. by making the open label green

Comment thread webviews/components/header.tsx
@Thomas1664

Copy link
Copy Markdown
Contributor

@daviddossett In #4052 I informed you that your PR breaks icon button focus outline, but you didn't take my concern serious: #4052 (comment)

After merging your changes from main into #4057 I noticed that my previously added support for focus is no longer visible. I hope that you take my concern seriously now and fix the issue that was caused by your PR.

@daviddossett
daviddossett merged commit 837b6fb into main Oct 14, 2022
@daviddossett
daviddossett deleted the ddossett/header-updates branch October 14, 2022 23:06
@daviddossett

Copy link
Copy Markdown
Contributor Author

@Thomas1664 where are you still seeing the issue? It sounded like it was fixed by this PR removing the icons from the header.

@Thomas1664

Copy link
Copy Markdown
Contributor

@Thomas1664 where are you still seeing the issue? It sounded like it was fixed by this PR removing the icons from the header.

In the comment action bar. But you have to check out #4057 because the action buttons weren't visible before on focus.

@daviddossett

Copy link
Copy Markdown
Contributor Author

I already fixed it in #4066

@alexr00

alexr00 commented Oct 17, 2022

Copy link
Copy Markdown
Member

I didn't have the time but I think we should replace the title editing form with a quick pick like how Settings Profile rename works. Thoughts?

I have a slight preference for leaving it as is. When you edit a comment in the overview, you get the text area. Keeping the title the same way is consistent.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants