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

Implement design system buttons #304

Merged

Conversation

steff456
Copy link
Contributor

@steff456 steff456 commented Sep 21, 2023

Part of #288

Description

This pull request:

  • Implements buttons following the design system

Main buttons

Default

image

Hover

image

Focus

image

Disabled

image

Icon buttons

Default

image

Hover

image

Focus

image

Toggle switch

Default

image

Hover / Focus

image

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Additional information

Screenshots

#### Sidebar

Before
image

After
image

View Environment

Before
image

After
image

Edit Environment

Before
image

image

After
image

image

Create Environment

Before
image

After
image

@steff456 steff456 added area: UI design 🎨 Items related to the user interface status: in progress 🏗 project: JATIC Work item needed for the JATIC project labels Sep 21, 2023
@steff456 steff456 self-assigned this Sep 21, 2023
@steff456 steff456 added this to the 🚀 JATIC - Q1 milestone Sep 21, 2023
@steff456 steff456 marked this pull request as ready for review September 21, 2023 17:33
@steff456 steff456 changed the title [WIP] Implement design system buttons Implement design system buttons Sep 21, 2023
Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Quick notes:

  • View environment: there is a slight shade difference (now) between the first environment and the rest. Is this a hover state? If so, this might need tweaking as the change is barely noticeable

  • Edit environment:

    • The Requested packages section has a green border - it should be gray
    • The toggle button (white) almost blends with the background. There is a shadow, but it is not enough to make it stand out. Can we add a border instead? @smeragoel
    • Same with the active env dropdown, this has an extra green border
  • There is not enough contrast on the disabled buttons (text vs background) - thought disabled buttons would be gray? @smeragoel

Colour_Contrast_Analyser__CCA__and_Implement_design_system_buttons_by_steff456_·Pull_Request__304·_conda-incubator_conda-store-ui

  • I am +1 on underlining links and items on focus - though the underline is correct against the text. Can we add an underlined offset?

This is what we have on PST, and it works well because it scales accordingly.

// Define some useful variables for links styling consistency
//
//  Thickness of the underline for links
// the default will be either:
//  - 1px
// - 0.0625rem if it's thicker than 1px because the user has changed the text
//   size in their browser
$link-underline-thickness: unquote("max(1px, .0625rem)") !default;
// Offset of link underlines from text baseline
// The default is 3px expressed as ems, as calculated against the default body
// font size (on desktop).
$link-underline-offset: 0.1578em !default;

This is based on the screenshots @steff456, as I only had a glance at the code. But from this quick glance it looks already much better in terms of maintainability ✨ thank you

@smeragoel
Copy link

smeragoel commented Sep 25, 2023

  • View environment: there is a slight shade difference (now) between the first environment and the rest. Is this a hover state? If so, this might need tweaking as the change is barely noticeable

I didn't add this component to the design system, but I will.

  • The toggle button (white) almost blends with the background. There is a shadow, but it is not enough to make it stand out. Can we add a border instead? @smeragoel

Okay, I will tweak the design so it's visible.

  • There is not enough contrast on the disabled buttons (text vs background) - thought disabled buttons would be gray? @smeragoel

They were supposed to be, but because of the greyscale design version, I used a lighter green. Let me work on a fix.

  • I am +1 on underlining links and items on focus - though the underline is correct against the text. Can we add an underlined offset?

I have added the offset to the designs as well!

@trallard
Copy link
Collaborator

I think we can keep the disabled buttons gray here since this is the "default theme" and deal with the gray theme as the customisation it is

@smeragoel
Copy link

Here's the new design for the toggle switch: https://www.figma.com/file/nKifk755TFMsK2SF082igS/conda-store-UI-design?type=design&node-id=1390%3A1571&mode=design&t=Pp6IQLDsLqaq4wps-1

Right now the toggle switch implies off/on states, which is not the most accurate signal to send, since we are just switching between interaction modes. I looked at examples from https://mui.com/material-ui/react-switch/ and used them as inspiration for the switch.

I also added clearer labels and interaction states. The icons (especially the gear for UI) and the content of the labels (GUI?) is up for discussion.

image

@trallard
Copy link
Collaborator

I also added clearer labels and interaction states. The icons (especially the gear for UI) and the content of the labels (GUI?) is up for discussion.

I like these a lot - I prefer the one with the < > as the gear sends my mind straight to settings-like which might not be entirely accurate 🤷🏽‍♀️

GUI and YAML seem perfectly fine as labels

@steff456
Copy link
Contributor Author

steff456 commented Oct 3, 2023

The latest commit fixes the disabled state and the hover style for buttons using the new guidelines.

It has some work in progress for the switch button, but this PR still needs more changes that I will work on after coming back from PTO.

@steff456
Copy link
Contributor Author

@trallard @smeragoel I just pushed the changes that address all the comments. I also updated the screenshots of all the views in the description and added the switch toggle states to it.

I just want to note that I'm unsure on the style of the dropdown menu style, so this PR only changed the style of the down arrow button that is part of the component. We can iterate on the style of the actual list in a follow up PR.

This PR is ready for another review!

@kcpevey
Copy link
Contributor

kcpevey commented Oct 27, 2023

I think this should be reviewed by @smeragoel before merging.

Copy link

@smeragoel smeragoel left a comment

Choose a reason for hiding this comment

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

One thing I noticed in the sidebar is that the bullets and the environment names are in green when it should be dark grey (secondary-800) by default.
image
Figma Link

@steff456
Copy link
Contributor Author

One thing I noticed in the sidebar is that the bullets and the environment names are in green when it should be dark grey (secondary-800) by default.

This PR is only focusing on the buttons, I'm currently working in a follow up PR for styling the sidebar as you described in the comment.

@costrouc costrouc merged commit 0d31098 into conda-incubator:design-system-implementation Oct 31, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI design 🎨 Items related to the user interface needs: changes 🧱 project: JATIC Work item needed for the JATIC project status: in progress 🏗
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

5 participants