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.

Sign up for GitHub

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

Jump to bottom

feat: UI overhaul #169

Merged
merged 7 commits into from Jul 25, 2022
Merged

feat: UI overhaul #169

merged 7 commits into from Jul 25, 2022

Conversation

jonathonherbert
Copy link
Contributor

@jonathonherbert jonathonherbert commented Jan 21, 2021 •
edited

What does this change?

A UI overhaul, including

  • more accessible matches
  • headers in the sidebar to separate and explain match types

They're based on this Figma file, with one conscious alteration – I've darkened the orange match colour to 300 to allow the icon colour to remain white in all cases.

How to test

These changes mostly concern the UI – there are no new unit tests. See success criteria.

Here's how it looks downstream in Composer, the Guardian's CMS:

Screenshot 2022-07-25 at 10 44 40

To test yourself in Composer, follow the instructions introduced in this PR.

How can we measure success?

Users find the new UI more accessible, the filter options are more clearly signposted, and it's clearer what each match colour means.

@jonathonherbert jonathonherbert marked this pull request as ready for review September 17, 2021 15:18
@jonathonherbert jonathonherbert changed the title feat: UI overhaul (draft) feat: UI overhaul Sep 17, 2021
@SHession
Copy link
Contributor

SHession commented Oct 12, 2021 •
edited

I'm finding the "Review" colour quite difficult to spot in context:
image

It's also failing the WCAG checks: https://colourcontrast.cc/ffffff/ffd32a

There is a slight wiggle on hover on "Review" decorations:
2021-10-12 16 58 46

I also think the underlines could do with being a bit thicker:
Current (4px height):
image

Proposed (20px width 6px height):
image

SHession
SHession reviewed Oct 13, 2021
View reviewed changes
Copy link
Contributor

@SHession SHession left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested locally in Composer and added a few comments to consider about the appearance and a few CODE suggestions.

src/ts/components/icons.tsx Show resolved Hide resolved
src/ts/components/SidebarMatches.tsx Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@jonathonherbert jonathonherbert changed the base branch from main to jsh/use-vite July 4, 2022 10:58
Base automatically changed from jsh/use-vite to main July 6, 2022 08:55
@jonathonherbert jonathonherbert force-pushed the jsh/more-underlines branch 2 times, most recently from e17f59c to d281213 Compare July 14, 2022 17:06
@jonathonherbert jonathonherbert changed the base branch from main to jsh/add-dev-mode July 14, 2022 17:07
jonathonherbert
jonathonherbert commented Jul 14, 2022
View reviewed changes
vite.config.ts Outdated
})
],
esbuild: {
logOverride: { "this-is-undefined-in-esm": "silent" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly necessary for now to ensure that our jsx transforms don't cause warnings in our output.

@jonathonherbert jonathonherbert force-pushed the jsh/add-dev-mode branch 2 times, most recently from 736c312 to 178083e Compare July 18, 2022 07:51
@jonathonherbert jonathonherbert changed the base branch from jsh/add-dev-mode to jsh/update-mui July 22, 2022 15:58
@jonathonherbert jonathonherbert mentioned this pull request Jul 22, 2022
Merged
Base automatically changed from jsh/update-mui to main July 25, 2022 11:24
rebecca-thompson
rebecca-thompson approved these changes Jul 25, 2022
View reviewed changes
Copy link
Contributor

@rebecca-thompson rebecca-thompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have tested this locally and it looks good to me

index.html Outdated
@@ -7,7 +7,7 @@
<meta http-equiv="X-UA-Compatible" content="ie=edge" />
<title>Document</title>
<script type="module" src="./pages/index.ts"></script>
<script src="https://unpkg.com/prosemirror-dev-tools@1.4.0/dist/umd/prosemirror-dev-tools.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for removing dev tools from the playground env?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No – thanks for spotting! Suspect I was on a high-latency connection and blocked by this script.

A good reminder to add this as a module and remove a networking dependency.

@jonathonherbert jonathonherbert merged commit 637002c into main Jul 25, 2022
@jonathonherbert jonathonherbert deleted the jsh/more-underlines branch July 25, 2022 12:51
@github-actions
Copy link

🎉 This PR is included in version 5.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jonathonherbert jonathonherbert mentioned this pull request Aug 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SHession SHession SHession left review comments

@rebecca-thompson rebecca-thompson rebecca-thompson approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jonathonherbert @SHession @rebecca-thompson