Create a reusable Label component #9833

Devessier · 2025-01-24T14:19:16Z

As seen with @Bonapara, I'm creating a Label component and use it in the workflows.

greptile-apps

PR Summary

This PR introduces a new reusable Label typography component with size variants and implements it in the workflow diagram, improving design system consistency.

New packages/twenty-ui/src/display/typography/components/Label.tsx component uses theme values for styling with 'default' (11px) and 'small' (9px) variants
Replaced custom styled div in WorkflowDiagramBaseStepNode.tsx with the new Label component for node type labels
Added comprehensive Storybook stories in Label.stories.tsx showcasing both variants with proper decorators
Consider adding text-transform and letter-spacing properties to match typical label styling patterns

_{4 file(s) reviewed, 4 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-01-24T14:20:13Z

packages/twenty-ui/src/display/typography/components/__stories__/Label.stories.tsx

+export const Default: Story = {
+  decorators: [ComponentDecorator],
+  args: {
+    children: 'Label',
+  },
+};


style: ComponentDecorator is redundant here since it's already defined in the meta object's decorators

greptile-apps · 2025-01-24T14:20:15Z

packages/twenty-ui/src/display/typography/components/Label.tsx

+  font-size: ${({ variant = 'default' }) => {
+    switch (variant) {
+      case 'default':
+        return '11px';
+      case 'small':
+        return '9px';
+    }
+  }};


logic: Switch statement missing default case. Could cause undefined font-size if invalid variant is passed.

greptile-apps · 2025-01-24T14:20:15Z

packages/twenty-ui/src/display/typography/components/Label.tsx

+
+export type LabelVariant = 'default' | 'small';
+
+const StyledLabel = styled.div<{ variant?: LabelVariant }>`


style: Consider using semantic HTML by extending 'label' or 'span' instead of 'div' for better accessibility

Question: isn't greptile right about this ? Or I've also misunderstood emotion styled here too

The label is not strictly meant to be used as part of a form. The benefit of exporting the component as-is is that it's possible to change the rendered element with the as prop.

greptile-apps · 2025-01-24T14:20:16Z

packages/twenty-ui/src/display/typography/components/Label.tsx

+  font-weight: ${({ theme }) => theme.font.weight.semiBold};
+`;
+
+export const Label = StyledLabel;


style: Component lacks TypeScript props interface definition for proper type checking of children and other potential props

martmull

One comment but lgtm

martmull · 2025-01-24T14:26:43Z

packages/twenty-ui/src/display/typography/components/Label.tsx

+  font-weight: ${({ theme }) => theme.font.weight.semiBold};
+`;
+
+export const Label = StyledLabel;


Suggested change

export const Label = StyledLabel;

export { StyledLabel as Label };

I don't know if there is a team choice on this topic, but I don't feel it provides more clarity. I changed it because I found another component that was exported similarly.

prastoin

Cool ! Will the Label styled component retrospectively applied elsewhere on the app or is it the first use case ?
Left few non-blocking comments

prastoin · 2025-01-24T14:58:55Z

packages/twenty-ui/src/display/typography/components/__stories__/Label.stories.tsx

+        {
+          name: 'Variant',
+          values: ['default', 'small'] as LabelVariant[],
+          props: (variant: LabelVariant) => ({ variant }),


Remark: Unless I'm mistaken force cast is not mandatory here, might wanna use satisfies instead if we wanna keep it tied to LabelVariant ?

Thanks for catching this. My as LabelVariant[] was pure bullshit.

prastoin · 2025-01-24T15:00:07Z

packages/twenty-ui/src/display/typography/components/Label.tsx

+
+export type LabelVariant = 'default' | 'small';
+
+const StyledLabel = styled.div<{ variant?: LabelVariant }>`


Question: isn't greptile right about this ? Or I've also misunderstood emotion styled here too

Devessier · 2025-01-24T17:41:42Z

@prastoin, this is a new component in the design system, as far as I understand.

Bonapara · 2025-01-24T17:54:55Z

Can we define its color to Text/light?

Devessier · 2025-01-24T18:21:56Z

Can we define its color to Text/light?

@Bonapara, I think this is what I did with theme.font.color.light. I checked, and it gives the same colors on light and dark mode than Text/light on Figma. Did I miss something?

github-actions · 2025-01-24T18:25:58Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-1f83f898.json

Generated by 🚫 dangerJS against a932390

feat: create a reusable label component and use in it the workflows

e6ec81c

greptile-apps bot reviewed Jan 24, 2025

View reviewed changes

Devessier added -PR: awaiting review -PR: awaiting CI run-e2e labels Jan 24, 2025

martmull approved these changes Jan 24, 2025

View reviewed changes

prastoin approved these changes Jan 24, 2025

View reviewed changes

Devessier added 2 commits January 24, 2025 18:42

refactor: use satisfies instead of as

180f444

refactor: change how component is exported

a932390

charlesBochet approved these changes Jan 24, 2025

View reviewed changes

charlesBochet merged commit 84c299f into main Jan 24, 2025
47 checks passed

charlesBochet deleted the add-label-component branch January 24, 2025 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a reusable Label component #9833

Create a reusable Label component #9833

Devessier commented Jan 24, 2025

greptile-apps bot left a comment

greptile-apps bot Jan 24, 2025

greptile-apps bot Jan 24, 2025

greptile-apps bot Jan 24, 2025

prastoin Jan 24, 2025

Devessier Jan 24, 2025

greptile-apps bot Jan 24, 2025

martmull left a comment

martmull Jan 24, 2025

Devessier Jan 24, 2025

prastoin left a comment •

edited

Loading

prastoin Jan 24, 2025 •

edited

Loading

Devessier Jan 24, 2025

prastoin Jan 24, 2025

Devessier commented Jan 24, 2025

Bonapara commented Jan 24, 2025

Devessier commented Jan 24, 2025

github-actions bot commented Jan 24, 2025


		export type LabelVariant = 'default' \| 'small';

		const StyledLabel = styled.div<{ variant?: LabelVariant }>`

	export const Label = StyledLabel;
	export { StyledLabel as Label };

Create a reusable Label component #9833

Create a reusable Label component #9833

Conversation

Devessier commented Jan 24, 2025

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jan 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 24, 2025

Choose a reason for hiding this comment

prastoin Jan 24, 2025

Choose a reason for hiding this comment

Devessier Jan 24, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 24, 2025

Choose a reason for hiding this comment

martmull left a comment

Choose a reason for hiding this comment

martmull Jan 24, 2025

Choose a reason for hiding this comment

Devessier Jan 24, 2025

Choose a reason for hiding this comment

prastoin left a comment • edited Loading

Choose a reason for hiding this comment

prastoin Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Devessier Jan 24, 2025

Choose a reason for hiding this comment

prastoin Jan 24, 2025

Choose a reason for hiding this comment

Devessier commented Jan 24, 2025

Bonapara commented Jan 24, 2025

Devessier commented Jan 24, 2025

github-actions bot commented Jan 24, 2025

Log

prastoin left a comment •

edited

Loading

prastoin Jan 24, 2025 •

edited

Loading