[feat] Add new themes #85

2Steaks · 2023-10-24T10:06:10Z

What?

This PR adds a rewrite of the UI for the ui package

Why?

We wanted to update the UIs & UX so that they feel more aligned to the k6 product.

Dark Mode

Light Mode

How to test?

move to /dashboard/assets/packages/ui
run yarn && yarn dev
open browser at suggested port
open another terminal in root of project
run go run mage.go run script.js
see the magic happen

Notes

@szkiba given how dynamic your framework is, would it be possible to add a one-off component for displaying a summary of stats (see screenshot of design) or does that break the model?

Checklist

I have performed a self-review of my code.
I have added tests for my changes.
I have run linter locally (go run mage.go lint) and all checks pass.
I have run tests locally (go run mage.go test) and all tests pass.
I have commented on my code, particularly in hard-to-understand areas.

- Update styles - Add new components - Add package react-popper (2.2kb gzip)

- Update build

# Conflicts: # dashboard/assets/packages/ui/dist/assets/index-2172ee12.js # dashboard/assets/packages/ui/dist/index.html

2Steaks · 2023-10-24T14:43:13Z

dashboard/assets/packages/ui/src/components/Chart/Chart.tsx

-  if (options.axes.length > 2) {
-    options.axes[2].side = 1
-  }
+  const options = createOptions({ plot, theme, width })

  function onCreate(chart: uPlot) {


@szkiba is this function still required? I'm yet to see it being used.

It is about to set second vertical axe to right side... if any

- Add hover state to IconButton - Add hover state to Menu.Item

ckbedwell

Big improvement and looks nice!

Predictably, you can guess what my comments are going to be about as I review all PRs using just my keyboard and screen reader 😬

Two other nits:

390px has a nice layout but once I go beneath that (or have browser zoom enabled), it reverts to multiple columns:

At certain resolutions the card metrics have uneven heights due to the titles wrapping to two lines. I think with a little bit of flex magic these could line up nicely still.

ckbedwell · 2023-10-26T12:46:10Z

dashboard/assets/packages/ui/src/components/ClickAwayListener.tsx

+
+  useEventListener("mouseup", handleClickAway, documentRef)
+
+  return <>{cloneElement(children, { ref: handleRef })}</>


Can we add a keyboard listener event to this as well so if you tab into or out of it it knows to close it? (And also 'esc' closes it 😄 ).

dashboard/assets/packages/ui/src/components/Icon/Icon.tsx

ckbedwell · 2023-10-26T12:48:52Z

dashboard/assets/packages/ui/src/components/Menu/Menu.tsx

+
+  return (
+    <>
+      <IconButton ref={setReferenceElement} name="options" variant="text" onClick={() => setIsOpen(!isOpen)} />


aria-label="Menu" here.

Given this some thought and I also think it should have aria-expanded={isOpen ? 'true' : ' false}.

dashboard/assets/packages/ui/src/components/Nav/Nav.tsx

dashboard/assets/packages/ui/src/components/Collapse/Collapse.tsx

ckbedwell · 2023-10-26T13:19:41Z

dashboard/assets/packages/ui/src/components/Nav/Nav.css.ts

+  active: [
+    itemBase,
+    {
+      color: vars.colors.text.primary
+    }
+  ],


In both light and dark modes I can barely see the active states. I think these could be improved so they are easier to identify.

I think you might have missed re-looking at this one 😃

Hmm I'd updated the theme and thought there was quite a strong difference now :/

Screen.Recording.2023-11-03.at.13.37.19.mov

It does match what you are showing there, I'm mostly finding the active and inactive states really similar. At a glance I can't differentiate between them without really concentrating. Could do something similar to what the Grafana tabs do with the underlines?

Having a line conflicts with the loading bar, how about a background?

ckbedwell

Few last comments! I missed a bit of cleanup from last time removing some of the redundant aria attributes and roles and gave a bit more thought to the menu.

I think you missed updating the active and hover states for the menu 😃

ckbedwell · 2023-11-03T11:39:40Z

dashboard/assets/packages/ui/src/components/Menu/Menu.tsx

+        <ClickAwayListener onClickAway={() => setIsOpen(false)}>
+          <Paper
+            {...attributes.popper}
+            aria-label="Menu"


Whoops, wrong place. This wants to be on the IconButton above rather than on the Paper component.

ckbedwell · 2023-11-03T12:01:15Z

dashboard/assets/packages/ui/src/App/App.tsx

-        ))}
-      </main>
-    </div>
+    <Flex direction="column" gap={3} role="tabpanel" id={`tabpanel-${idx}`} aria-labelledby={`tab-${idx}`}>


Sorry, I missed these the first time round. You can drop the role, id and aria-labelledby attributes here, they aren't hooked up correctly and I think in the context of this app they aren't really tabs.

ckbedwell · 2023-11-03T12:02:24Z

dashboard/assets/packages/ui/src/components/Nav/Nav.css.ts

+  active: [
+    itemBase,
+    {
+      color: vars.colors.text.primary
+    }
+  ],


I think you might have missed re-looking at this one 😃

ckbedwell · 2023-11-03T12:03:23Z

dashboard/assets/packages/ui/src/components/Nav/Nav.tsx

+      {options.map((option, index) => (
+        <Item
+          key={option.id}
+          aria-controls={`nav-${index}`}


Can remove the aria-controls here as it isn't hooked up correctly and as said for the tab panel, I think isn't the right tool to use in this context.

ckbedwell · 2023-11-03T12:12:58Z

dashboard/assets/packages/ui/src/components/Menu/Menu.tsx

+
+  return (
+    <>
+      <IconButton ref={setReferenceElement} name="options" variant="text" onClick={() => setIsOpen(!isOpen)} />


Given this some thought and I also think it should have aria-expanded={isOpen ? 'true' : ' false}.

ckbedwell

Awesome job, looks great! 💪

[feat] Add new themes

41e23f9

- Update styles - Add new components - Add package react-popper (2.2kb gzip)

2Steaks self-assigned this Oct 24, 2023

2Steaks added 2 commits October 24, 2023 11:43

[feat] Add Table component

0365f73

- Update build

Merge branch 'master' into feat/ui-theme

d0efc2e

# Conflicts: # dashboard/assets/packages/ui/dist/assets/index-2172ee12.js # dashboard/assets/packages/ui/dist/index.html

2Steaks requested review from szkiba and ckbedwell October 24, 2023 10:45

2Steaks commented Oct 24, 2023

View reviewed changes

2Steaks added 8 commits October 24, 2023 15:45

[fix] Remove console log

dd2c41c

[refactor] Clean up redundant code

9f5a20e

- Add hover state to IconButton - Add hover state to Menu.Item

[fix] Fix IconButton type

179ae86

[refactor] Move animation into theme

0a15556

[fix] Update main content padding

01c7f81

[fix] Improve visibility of light mode

126b7fd

[fix] Revert colour change

144ca7d

[fix] Improve text hover state for light mode

ddff7b4

ckbedwell suggested changes Oct 26, 2023

View reviewed changes

szkiba and others added 3 commits October 26, 2023 15:28

chore: added license headers, and generate assets for go

7abc6b8

[fix] Improved accessibility and fixed layout

c9de71b

[fix] Update dark mode colours

779bc6e

2Steaks requested a review from ckbedwell October 31, 2023 10:39

ckbedwell reviewed Nov 3, 2023

View reviewed changes

2Steaks added 2 commits November 3, 2023 13:50

[fix] Update accessibility props

81c3ff7

[fix] Improve nav state visibility

2834f8a

2Steaks requested a review from ckbedwell November 6, 2023 11:29

[fix] Improved layout for Timings page

ea13823

ckbedwell approved these changes Nov 6, 2023

View reviewed changes

2Steaks merged commit 4a2e03f into master Nov 6, 2023
10 checks passed

szkiba deleted the feat/ui-theme branch June 7, 2024 11:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat] Add new themes #85

[feat] Add new themes #85

2Steaks commented Oct 24, 2023 •

edited

Loading

2Steaks Oct 24, 2023 •

edited

Loading

szkiba Oct 26, 2023

ckbedwell left a comment

ckbedwell Oct 26, 2023

ckbedwell Oct 26, 2023

ckbedwell Nov 3, 2023

ckbedwell Oct 26, 2023

ckbedwell Nov 3, 2023

2Steaks Nov 3, 2023

ckbedwell Nov 3, 2023

2Steaks Nov 6, 2023 •

edited

Loading

ckbedwell left a comment

ckbedwell Nov 3, 2023

ckbedwell Nov 3, 2023

ckbedwell Nov 3, 2023

ckbedwell Nov 3, 2023

ckbedwell Nov 3, 2023

ckbedwell left a comment


		useEventListener("mouseup", handleClickAway, documentRef)

		return <>{cloneElement(children, { ref: handleRef })}</>

[feat] Add new themes #85

[feat] Add new themes #85

Conversation

2Steaks commented Oct 24, 2023 • edited Loading

What?

Why?

Dark Mode

Light Mode

How to test?

Notes

Checklist

2Steaks Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckbedwell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2Steaks Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

ckbedwell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckbedwell left a comment

Choose a reason for hiding this comment

2Steaks commented Oct 24, 2023 •

edited

Loading

2Steaks Oct 24, 2023 •

edited

Loading

2Steaks Nov 6, 2023 •

edited

Loading