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.

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

Upgrade design system #9253

Draft
wants to merge 58 commits into
base: main
Choose a base branch
from
Draft

Upgrade design system #9253

wants to merge 58 commits into from

Conversation

dcousens
Copy link
Member

@dcousens dcousens commented Aug 5, 2024

(this is a draft)

@gautamsi
Copy link
Member

gautamsi commented Aug 5, 2024

@dcousens
I believe this is a blocker for #9186 ?
Do you have any figma/design link we can provide feedback on ? is this going to refresh only the design elements or the design of the page overall? like the hints provided 2 years ago in a developer talk?

@dcousens dcousens self-assigned this Aug 5, 2024
@kennedybaird
Copy link
Contributor

@jossmac - How do you test for a wide range of fields, do you run the sandbox with "all the things"?

We used a Status select which made everything read only once past "draft". This showed a bunch of UX that could be improved - they may seem as less common use-cases, but I have noticed a lot of projects that use in this way (and have done myself a few times).

image
image

  • null values on End Date, Reviewer
  • disabled drop down on User, Reviewer, Tags
  • Poorly used space with "View User details"

--

We could add a simple checkbox to make them all read only in the sandbox project - wdyt @dcousens?

@dcousens
Copy link
Member Author

@kennedybaird I think adding a checkbox (or cookie, or) that can easily switch between different field modes will be incredibly helpful in debugging UX

@kennedybaird
Copy link
Contributor

@kennedybaird I think adding a checkbox (or cookie, or) that can easily switch between different field modes will be incredibly helpful in debugging UX

@dcousens - to clarify, you'd like this to expand the current tests/sandbox, or a new project in tests/test-projects?

@dcousens
Copy link
Member Author

tests/sandbox, but I think we can defer to @jossmac to know if that is useful in this pull request, or separately

@dcousens
Copy link
Member Author

dcousens commented Aug 14, 2024

@gautamsi

I believe this is a blocker for #9186?

Yes, I think this is blocking

Do you have any figma/design link we can provide feedback on?

Unfortunately, the design work completed in 2022 and 2023 (that we intended to make public) was superseded by work in Keystatic

@gautamsi
Copy link
Member

@jossmac I am sure you have noticed my work in #9186
We should move in the direction of SSR friendly design so that any SSR only rendering with NextJs 15 and react 19 should not require major rework in this regards.
They are around the corner and my idea is to move towards App router with maximizing SSR use in admin ui.

@@ -1,43 +1,9 @@
/** @jsxRuntime classic */
/** @jsx jsx */

import 'intersection-observer'
Copy link
Member

Choose a reason for hiding this comment

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

can we add 'use client' here, this is harmless in non ssr but useful in SSR with upcoming changes.

Copy link
Member Author

@dcousens dcousens Aug 20, 2024

Choose a reason for hiding this comment

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

@gautamsi any reason not to do this in the upcoming changes rather than this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I can do that

Copy link
Member

Choose a reason for hiding this comment

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

Totally on board with SSR friendly! This PR is pretty massive in scope, let's focus on app-router etc. after it's landed.

Copy link
Member

Choose a reason for hiding this comment

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

Looking forward to get this design uplift, would love to see this coming. wait is killing!!

@dcousens dcousens force-pushed the introduce-keystar-ui branch 2 times, most recently from aa587b9 to 2c0b220 Compare August 20, 2024 07:47
@jossmac
Copy link
Member

jossmac commented Sep 12, 2024

Known issues:

  • keystone-ui component instances still litter more than a few corners of the repo
  • ui within packages outside "core" haven't been reviewed
  • relationship field was a mess, best efforts to update ui but i'm certain there's issues
  • multi-select doesn't have filters
  • list item selection dissonance between ALL and all in page
  • list initialSort isn't respected
  • some docs will be incorrect after this lands

Potential improvements:

  • the cell contents of numeric fields (decimal, float, integer), and maybe temporal fields (timestamp,calendarDay) should be right aligned
  • filter label should own complete render, allowing things like "not published" instead of "published is false"
  • support user controlled cell density—needs UI and storage implementation
  • support consumer controlled initial column widths—needs implementation
  • support user controlled column widths—needs storage implementation
  • support user controlled color-scheme—this might need to be prioritised, since old UI portions could be visually inaccessible depending on user preferences
  • filter tags offer poor UX on mobile devices; consider something like AirTable's approach with a dedicated dialog. could also be an opportunity to introduce AND/OR functionality
  • introduce item "actions" concept, which may solve consumer hacks

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.

4 participants