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

v5 component refactor: Label #272

Open
11 tasks
ksolanki7 opened this issue Jan 7, 2025 · 5 comments
Open
11 tasks

v5 component refactor: Label #272

ksolanki7 opened this issue Jan 7, 2025 · 5 comments
Assignees
Labels
feature The issue relates to a new feature v5 Issues for v5 release

Comments

@ksolanki7
Copy link
Contributor

Abstract

As part of the v5 Elements release, each component will be reviewed and refactored to ensure best practice and design system alignment

Specification

Developer Checklist

  • Styles alignment between Design System and Elements
  • Check design tokens in Figma and implement CSS variable tokens if available for relevant component
  • Align with accessibility standards / spec as per above
  • If relevant, break down component into Styles Only and React component structures
  • Ensure all variants of components are documented as appropriate
  • Ensure unit test coverage is adequate for component
  • Update documentation in MDX file as per guidelines
  • Changelog updated to reflect a single beta version per component ideally

Release Checklist

  • Approved PR merged to main
  • Design & product review and feedback addressed by developer
  • Beta release by product / engineering lead to next beta version
@ksolanki7 ksolanki7 added feature The issue relates to a new feature v5 Issues for v5 release labels Jan 7, 2025
@github-project-automation github-project-automation bot moved this to To Review in Elements Roadmap Jan 7, 2025
@ksolanki7
Copy link
Contributor Author

@HollyJoyPhillips I have created this issue for new element Label which dev ready over figma.

@ksolanki7 ksolanki7 moved this from To Review to Todo in Elements Roadmap Jan 7, 2025
@rpt-rfoxy
Copy link

rpt-rfoxy commented Jan 7, 2025

Hello Genks,
cc @kurtdoherty && @ksolanki7

From the Figma design, the new Label design only requires adding new parameters, Here's my proposed solution:


Updates to the Label Component

1. UI Updates

  • No new design is required, only two variants will be added.
  • add new two size (--font-size-xs and --font-size-sm )
  • add * mark as required info

2. New Parameters

Label Solution Design

The Label will have the following main prop interface

Prop Value Default Usage
variant?: union "soft" , "strong", undefined soft label variant depend on text color
size?: union "small" , "medium", undefined medium label size using --font-size-xs and --font-size-sm
required?: boolean true, false, undefined undefined to set * mark in the end of element child

  • Text Overflow
    By default, the text overflow will continue on a new line, depending on the wrapper.

Code Concept

to see the detail implementation, you can check it here (POC) please let me know if the link didnt work

// types.ts
export interface LabelProps extends LabelHTMLAttributes<HTMLLabelElement> {
  variant?: "soft" | "strong";
  size?: "small" | "medium";
  required?: boolean;
}
// Label.tsx
export const Label: FC<LabelProps> = ({
  children,
  required,
  size = "medium",
  variant = "soft",
  ...rest
}) => {
  return (
    <ElLabel {...rest} data-size={size} data-variant={variant}>
      {children}
      {required && " *"}
    </ElLabel>
  );
};
//style.ts

export const ElLabel = styled.label`
  &[data-size="small"] {
    font-size: var(--font-size-xs);
  }
  &[data-size="medium"] {
    font-size: var(--font-size-sm);
  }
  &[data-variant="soft"] {
    color: var(--neutral-500);
  }
  &[data-variant="strong"] {
    color: var(--text-primary);
  }
`;

@ksolanki7
Copy link
Contributor Author

praise: Thank you @rpt-rfoxy for coming up with the solution for your 1st ticket.

suggestion: It is always a good idea to deprecate the old component and create a new one. Here's why:

  • Maintain consistency: Upgrading a design system aligns it with updated standards while deprecating old components ensures consistency and a cohesive user experience.
  • Simplify Maintenance: Deprecating outdated components reduces complexity and technical debt, simplifying maintenance and evolution of the design system.
  • Best practice and QA: Deprecating older components encourage the adoption of improved, modern ones with better accessibility, performance, and usability, reducing regressions and simplifying testing.

note: In terms of UI updates, there are differences in the color, font, and other CSS properties in the new DS for the label component, so highly recommend deprecating the existing v4 label and creating a new v5 label.

suggestion: I would personally recommend using variant or a similar approach instead of intent to align with other v5 components.

suggestion: It would be great if the solution document includes what actual props will be used and their role in the component. Check this for ref: #242

suggestion: I believe using a data attribute provides a more component-centric and semantic approach compared to class names. For instance, if the React component prop is size, the corresponding styled component could utilize a data-size attribute. The styles would then target elements with selectors like &[data-size="small"] { ... } or &[data-size="medium"] { ... }. In contrast, class names such as elSizeSmall and elSizeMedium can feel overly generic and less tied to the specific component context.

@rpt-rfoxy
Copy link

rpt-rfoxy commented Jan 8, 2025

praise: hi @ksolanki7 thanks for the feedback for the Design solution.
note: i already update the Design Solution, and i add the link to the POC too, please let me know if there is anything to update

@ksolanki7
Copy link
Contributor Author

ksolanki7 commented Jan 8, 2025

@rpt-rfoxy praise Thanks for updating the solution.

suggestion: From the POC - It is unnecessary to create a separate file for types when there are only a few interfaces and not shared. Including them directly within the component file is a more streamlined approach, enhancing readability and maintaining context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature The issue relates to a new feature v5 Issues for v5 release
Projects
Status: Todo
Development

No branches or pull requests

2 participants