Skip to content

feat(utility-classes): add CSS helper classes#2576

Open
veekays wants to merge 1 commit intoinnovaccer:developfrom
veekays:feat-utility-classes
Open

feat(utility-classes): add CSS helper classes#2576
veekays wants to merge 1 commit intoinnovaccer:developfrom
veekays:feat-utility-classes

Conversation

@veekays
Copy link
Collaborator

@veekays veekays commented May 5, 2025

What does this implement/fix? Explain your changes.

...

Does this close any currently open issues?

...

Any other comments?

...

Dependent PRs/Commits

...

Describe breaking changes, if any.

...

Checklist

Check all those that are applicable and complete.

  • Merged with latest master branch

@veekays veekays force-pushed the feat-utility-classes branch from 21135d3 to 309cc06 Compare May 5, 2025 17:30
Copy link
Collaborator

@anuradha9712 anuradha9712 left a comment

Choose a reason for hiding this comment

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

Please remove classname text-muted as it does not exist in our design system

@anuradha9712
Copy link
Collaborator

anuradha9712 commented May 6, 2025

Please add some styling to the classname as currently classname and their corresponding examples are indistinguishable

image

Comment on lines 88 to 90
<Paragraph appearance="default">
Use border utilities to add or remove an element's borders. Choose from all borders or one at a time.
</Paragraph>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason for updating Text with Paragraph as it does not resemble with the rest of the stories on storybook??

Comment on lines 217 to 256
{/* Border radius examples */}
<div className="mt-8 mb-8">
<Heading size="s" className="mb-3">
Border Radius
</Heading>
<div className="mb-4 d-flex flex-wrap gap-5">
<div>
<div className="text-center mb-2">rounded-2-5</div>
<div className="border rounded-2-5 w-30 h-30 bg-stone-lightest d-flex align-items-center justify-content-center">
1px radius
</div>
</div>
<div>
<div className="text-center mb-2">rounded-05</div>
<div className="border rounded-05 w-30 h-30 bg-jal-lightest d-flex align-items-center justify-content-center">
2px radius
</div>
</div>
<div>
<div className="text-center mb-2">rounded-10</div>
<div className="border rounded-10 w-30 h-30 bg-neem-lightest d-flex align-items-center justify-content-center">
4px radius
</div>
</div>
<div>
<div className="text-center mb-2">rounded-20</div>
<div className="border rounded-20 w-30 h-30 bg-haldi-lightest d-flex align-items-center justify-content-center">
8px radius
</div>
</div>
<div>
<div className="text-center mb-2">rounded-full</div>
<div className="border rounded-full w-30 h-30 bg-jamun-lightest d-flex align-items-center justify-content-center">
Full radius
</div>
</div>
</div>
<div className="mb-2 color-text-subtle font-size-xs">
These utilities help you apply different border radius values to your elements.
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please show example for all the existing border radius classes and not just for the few of them.

image

Comment on lines 41 to 90
/* Border color utilities */
.border-primary {
border-color: var(--primary) !important;
}

.border-secondary {
border-color: var(--secondary) !important;
}

.border-success {
border-color: var(--success) !important;
}

.border-warning {
border-color: var(--warning) !important;
}

.border-alert {
border-color: var(--alert) !important;
}

.border-accent1 {
border-color: var(--accent1) !important;
}

.border-accent2 {
border-color: var(--accent2) !important;
}

.border-accent3 {
border-color: var(--accent3) !important;
}

.border-accent4 {
border-color: var(--accent4) !important;
}

/* Border style utilities */
.border-dashed {
border-style: dashed !important;
}

.border-solid {
border-style: solid !important;
}

.border-dotted {
border-style: dotted !important;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we are not showing examples for all these classes on storybook?

Comment on lines +79 to +89
.border-dashed {
border-style: dashed !important;
}

.border-solid {
border-style: solid !important;
}

.border-dotted {
border-style: dotted !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add classes for border style inset and outset as well.

Comment on lines 62 to 76
.border-accent1 {
border-color: var(--accent1) !important;
}

.border-accent2 {
border-color: var(--accent2) !important;
}

.border-accent3 {
border-color: var(--accent3) !important;
}

.border-accent4 {
border-color: var(--accent4) !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we consider to update this classes with border-accent-1, border-accent-2, etc.


.rounded-2-5 {
border-radius: var(--border-radius-2-5) !important;
}
Copy link
Collaborator

@anuradha9712 anuradha9712 May 6, 2025

Choose a reason for hiding this comment

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

We have tokens defined for border-width here. Please add classes for the same.

Comment on lines 85 to 105
<div className="d-inline bg-stone-lightest border border-2 border-stone-light p-2 px-4 mr-2 mb-2 align-middle lh-8">
d-inline
</div>
<div className="d-inline bg-jal-lightest border border-2 border-jal-light p-2 px-4 mr-2 mb-2 align-middle lh-8">
d-inline
</div>
<div className="d-inline bg-neem-lightest border border-2 border-neem-light p-2 px-4 mr-2 mb-2 align-middle lh-8">
d-inline
</div>
<div className="d-inline bg-tawak-lightest border border-2 border-tawak-light p-2 px-4 mr-2 mb-2 align-middle lh-8">
d-inline
</div>
<div className="d-inline bg-mirch-lightest border border-2 border-mirch-light p-2 px-4 mr-2 mb-2 align-middle lh-8">
d-inline
</div>
</div>
<div className="mb-2 text-muted color-text-subtle font-size-xs">
Elements with d-inline display as inline elements, appearing side by side and taking up only as much width as
necessary. They will wrap to new lines when they reach the container's edge.
</div>
<div className="bg-stone-lightest p-3 rounded-10 font-monospace font-size-xs">
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove all these unused classes which does not even exist in our design system like:

  • text-muted
  • bg-stone-lightest
  • border-2
  • border-stone-light
  • lh-8
  • bg-jal-lightest
  • border-jal-light
  • bg-neem-lightest
  • border-neem-light
  • bg-tawak-lightest
  • border-tawak-light
  • bg-mirch-lightest
  • border-mirch-light
  • color-text-subtle
  • bg-stone-lightest

And also it does not make any sense to use p-2 px-4 together.

Comment on lines 101 to 104
<div className="mb-2 text-muted color-text-subtle font-size-xs">
Elements with d-inline display as inline elements, appearing side by side and taking up only as much width as
necessary. They will wrap to new lines when they reach the container's edge.
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This text should be wrapped inside some Typography component.

@veekays veekays force-pushed the feat-utility-classes branch 2 times, most recently from 99233a7 to f381c35 Compare May 7, 2025 14:05
}}
className="mt-5 mb-8"
>
<Text>Use border utilities to add or remove an element's borders. Choose from all borders or one at a time.</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add weight="strong" to Text component to make it consistent with all other stories

Comment on lines 42 to 76
.border-primary {
border-color: var(--primary) !important;
}

.border-secondary {
border-color: var(--secondary) !important;
}

.border-success {
border-color: var(--success) !important;
}

.border-warning {
border-color: var(--warning) !important;
}

.border-alert {
border-color: var(--alert) !important;
}

.border-accent1 {
border-color: var(--accent1) !important;
}

.border-accent2 {
border-color: var(--accent2) !important;
}

.border-accent3 {
border-color: var(--accent3) !important;
}

.border-accent4 {
border-color: var(--accent4) !important;
}
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Please add border-color classes for all the color-tokens available in our design system as there are multiple use-cases for using lighter and lightest variant of colors in case of border.

Comment on lines 234 to 248
<Text appearance="default" className="border bg-secondary-lightest p-8 d-inline-block mr-6">
border
</Text>
<Text appearance="default" className="border-top bg-secondary-lightest p-8 d-inline-block mr-6">
top
</Text>
<Text appearance="default" className="border-right bg-secondary-lightest p-8 d-inline-block mr-6">
right
</Text>
<Text appearance="default" className="border-bottom bg-secondary-lightest p-8 d-inline-block mr-6">
bottom
</Text>
<Text appearance="default" className="border-left bg-secondary-lightest p-8 d-inline-block">
left
</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Applying border classes directly to typography components like Text is generally not appropriate. Borders are better suited for layout or wrapper components that handle visual structure rather than text styling.

Comment on lines 254 to 275
'<Text appearance="default" className="border bg-secondary-lightest p-8 d-inline-block mr-6">border</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-top bg-secondary-lightest p-8 d-inline-block mr-6">top</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-right bg-secondary-lightest p-8 d-inline-block mr-6">right</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-bottom bg-secondary-lightest p-8 d-inline-block mr-6">bottom</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-left bg-secondary-lightest p-8 d-inline-block">left</Text>'
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead.

Comment on lines 290 to 305
<Text appearance="default" className="border border-0 bg-secondary-lightest p-8 d-inline-block mr-6">
border-0
</Text>
<Text appearance="default" className="border border-top-0 bg-secondary-lightest p-8 d-inline-block mr-6">
top-0
</Text>
<Text appearance="default" className="border border-right-0 bg-secondary-lightest p-8 d-inline-block mr-6">
right-0
</Text>
<Text appearance="default" className="border border-bottom-0 bg-secondary-lightest p-8 d-inline-block mr-6">
bottom-0
</Text>
<Text appearance="default" className="border border-left-0 bg-secondary-lightest p-8 d-inline-block">
left-0
</Text>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead.

Comment on lines 315 to 331
'<Text appearance="default" className="border border-top-0 bg-secondary-lightest p-8 d-inline-block mr-6">top-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-right-0 bg-secondary-lightest p-8 d-inline-block mr-6">right-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-bottom-0 bg-secondary-lightest p-8 d-inline-block mr-6">bottom-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-left-0 bg-secondary-lightest p-8 d-inline-block">left-0</Text>'
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead.

Comment on lines 346 to 369
<Text appearance="default" className="border rounded-2-5 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-2-5
</Text>
<Text appearance="default" className="border rounded-05 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-05
</Text>
<Text appearance="default" className="border rounded-10 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-10
</Text>
<Text appearance="default" className="border rounded-15 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-15
</Text>
<Text appearance="default" className="border rounded-20 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-20
</Text>
<Text appearance="default" className="border rounded-30 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-30
</Text>
<Text appearance="default" className="border rounded-40 bg-secondary-lightest p-8 d-inline-block mr-6">
rounded-40
</Text>
<Text appearance="default" className="border rounded-full bg-secondary-lightest p-8 d-inline-block">
rounded-full
</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead for better separation of concern

Comment on lines 252 to 271
<code className="d-block">
{
'<Text appearance="default" className="border bg-secondary-lightest p-8 d-inline-block mr-6">border</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-top bg-secondary-lightest p-8 d-inline-block mr-6">top</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-right bg-secondary-lightest p-8 d-inline-block mr-6">right</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border-bottom bg-secondary-lightest p-8 d-inline-block mr-6">bottom</Text>'
}
</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements we can simply write all the text inside single <code>

Comment on lines 307 to 330
<pre className="m-0">
<code className="d-block">
{
'<Text appearance="default" className="border border-0 bg-secondary-lightest p-8 d-inline-block mr-6">border-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-top-0 bg-secondary-lightest p-8 d-inline-block mr-6">top-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-right-0 bg-secondary-lightest p-8 d-inline-block mr-6">right-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-bottom-0 bg-secondary-lightest p-8 d-inline-block mr-6">bottom-0</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-left-0 bg-secondary-lightest p-8 d-inline-block">left-0</Text>'
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements, it's better to place all the text within a single <code> block.

Comment on lines 372 to 413
<pre className="m-0">
<code className="d-block">
{
'<Text appearance="default" className="border rounded-2-5 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-2-5</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-05 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-05</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-10 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-10</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-15 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-15</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-20 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-20</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-30 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-30</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-40 bg-secondary-lightest p-8 d-inline-block mr-6">rounded-40</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border rounded-full bg-secondary-lightest p-8 d-inline-block">rounded-full</Text>'
}
</code>
</pre>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements, it's better to place all the text within a single <code> block.

Comment on lines 426 to 453
<Text appearance="default" className="border border-primary bg-white p-8 d-inline-block mr-6">
border-primary
</Text>
<Text appearance="default" className="border border-secondary bg-white p-8 d-inline-block mr-6">
border-secondary
</Text>
<Text appearance="default" className="border border-success bg-white p-8 d-inline-block mr-6">
border-success
</Text>
<Text appearance="default" className="border border-warning bg-white p-8 d-inline-block mr-6">
border-warning
</Text>
<Text appearance="default" className="border border-alert bg-white p-8 d-inline-block mr-6">
border-alert
</Text>
<Text appearance="default" className="border border-accent1 bg-white p-8 d-inline-block mr-6">
border-accent1
</Text>
<Text appearance="default" className="border border-accent2 bg-white p-8 d-inline-block mr-6">
border-accent2
</Text>
<Text appearance="default" className="border border-accent3 bg-white p-8 d-inline-block mr-6">
border-accent3
</Text>
<Text appearance="default" className="border border-accent4 bg-white p-8 d-inline-block">
border-accent4
</Text>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead.

Comment on lines 455 to 476
<pre className="m-0">
<code className="d-block">
{
'<Text appearance="default" className="border border-primary bg-white p-8 d-inline-block mr-6">border-primary</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-secondary bg-white p-8 d-inline-block mr-6">border-secondary</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-success bg-white p-8 d-inline-block mr-6">border-success</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-alert bg-white p-8 d-inline-block mr-6">border-alert</Text>'
}
</code>
</pre>
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements we can simply write all the text inside single <code>

Comment on lines 506 to 532
<pre className="m-0">
<code className="d-block">
{
'<Text appearance="default" className="border border-solid bg-secondary-lightest p-8 d-inline-block mr-6">border-solid</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-dashed bg-secondary-lightest p-8 d-inline-block mr-6">border-dashed</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-dotted bg-secondary-lightest p-8 d-inline-block mr-6">border-dotted</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-inset bg-secondary-lightest p-8 d-inline-block mr-6">border-inset</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="border border-outset bg-secondary-lightest p-8 d-inline-block">border-outset</Text>'
}
</code>
</pre>
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements we can simply write all the text inside single <code>

Comment on lines 489 to 503
<Text appearance="default" className="border border-solid bg-secondary-lightest p-8 d-inline-block mr-6">
border-solid
</Text>
<Text appearance="default" className="border border-dashed bg-secondary-lightest p-8 d-inline-block mr-6">
border-dashed
</Text>
<Text appearance="default" className="border border-dotted bg-secondary-lightest p-8 d-inline-block mr-6">
border-dotted
</Text>
<Text appearance="default" className="border border-inset bg-secondary-lightest p-8 d-inline-block mr-6">
border-inset
</Text>
<Text appearance="default" className="border border-outset bg-secondary-lightest p-8 d-inline-block">
border-outset
</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove border classes from the Text, use layout or wrapper components instead.

<Heading size="xxl">Display Utilities</Heading>
<br />
<Text weight="strong">
<Text appearance="default">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Passing appearance="default" to the Text component is unnecessary, as it is the default value. Also add weight="strong" to Text.

Comment on lines 107 to 116
<code className="d-block">
{
'<Text appearance="default" className="d-inline bg-secondary-lightest border p-4 mr-2 mb-2 align-middle">d-inline</Text>'
}
</code>
<code className="d-block mt-2">
{
'<Text appearance="default" className="d-inline bg-secondary-lightest border p-4 mr-2 mb-2 align-middle">d-inline</Text>'
}
</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using multiple <code> elements we can simply write all the text inside single <code>

Comment on lines 207 to 220
<div className="bg-secondary-lightest p-3 rounded-10 font-size-xs">
<pre className="m-0">
<code className="d-block">
{'<div className="d-inline-block bg-secondary-lightest border p-3 mr-2 mb-2 w-25 h-50 align-top">'}
</code>
<code className="d-block mt-2">{' d-inline-block'}</code>
<code className="d-block mt-2">{' <br />'}</code>
<code className="d-block mt-2">{' w-25 class'}</code>
<code className="d-block mt-2">{' <br />'}</code>
<code className="d-block mt-2">{' h-50 class'}</code>
<code className="d-block mt-2">{'</div>'}</code>
</pre>
</div>
</div>
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Write full code of the example to avoid confusion. Also Instead of using multiple <code> elements we can simply write all the text inside single <code>

Copy link
Collaborator

@anuradha9712 anuradha9712 left a comment

Choose a reason for hiding this comment

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

font-size-xs classname does not exist in our design system, Please remove it from everywhere

Comment on lines 288 to 295
<Text appearance="default" className="d-table-cell w-33 p-3 bg-secondary-lightest border">
Table cell 1
</Text>
<Text appearance="default" className="d-table-cell w-33 p-3 bg-secondary-lightest border">
Table cell 2
</Text>
<Text appearance="default" className="d-table-cell w-33 p-3 bg-secondary-lightest border">
Table cell 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove w-33 classname as it does not exist in our design system

Comment on lines 463 to 481
<div className="d-flex gap-3 Utilities-example">
<div className="p-2">Flex item with gap</div>
<div className="p-2">Flex item with gap</div>
<div className="p-2">Flex item with gap</div>
</div>
&nbsp;
<div className="DocPage-codeBlock pb-5 pt-5 pl-5">
<code>
{'<div className="d-flex gap-3">'}
<br />
&nbsp; {'<div className="p-2">Flex item with gap</div>'}
<br />
&nbsp; {'<div className="p-2">Flex item with gap</div>'}
<br />
&nbsp; {'<div className="p-2">Flex item with gap</div>'}
<br />
{'</div>'}
<br />
</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use some other examples where gap between items are more visible
image

Comment on lines 179 to 188
/* New utility classes */
{
className: 'gap-0',
properties: 'gap: 0 ;',
},
{
className: 'gap-1',
properties: 'gap: var(--spacing-2-5) ; /* 1px */',
},
{
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Please create a separate table for each group of class for better user discoverability like different tables for following group of classes:

  • flex
  • justify-content
  • align-items
  • align-content
  • align-self
  • order
  • gap, row-gap, column-gap
  • justify-items
  • justify-self
  • place-content
  • place-items
  • place-self

Comment on lines 174 to 196
.gap-0 {
gap: 0 !important;
}

.gap-1 {
gap: var(--spacing-2-5) !important;
}

.gap-2 {
gap: var(--spacing-05) !important;
}

.gap-3 {
gap: var(--spacing-10) !important;
}

.gap-4 {
gap: var(--spacing-20) !important;
}

.gap-5 {
gap: var(--spacing-60) !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow our predefined guidelines for defining the tokens and classnames. The correct classnames for gap would be using the formula (x / 4) * 100:

  • gap-2-5: 1px
  • gap-05: 2px
  • gap-7-5: 3px
  • gap-10: 4px
  • gap-15: 6px
  • gap-20: 8px
  • gap-30: 12px

Comment on lines 199 to 221
.row-gap-0 {
row-gap: 0 !important;
}

.row-gap-1 {
row-gap: var(--spacing-2-5) !important;
}

.row-gap-2 {
row-gap: var(--spacing-05) !important;
}

.row-gap-3 {
row-gap: var(--spacing-10) !important;
}

.row-gap-4 {
row-gap: var(--spacing-20) !important;
}

.row-gap-5 {
row-gap: var(--spacing-60) !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow our predefined guidelines for defining the tokens and classnames. The correct classnames for row-gap would be using the formula (x / 4) * 100 as describes in the above comment. Please take reference from the border classes

Comment on lines 224 to 246
.column-gap-0 {
column-gap: 0 !important;
}

.column-gap-1 {
column-gap: var(--spacing-2-5) !important;
}

.column-gap-2 {
column-gap: var(--spacing-05) !important;
}

.column-gap-3 {
column-gap: var(--spacing-10) !important;
}

.column-gap-4 {
column-gap: var(--spacing-20) !important;
}

.column-gap-5 {
column-gap: var(--spacing-60) !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow our predefined guidelines for defining the tokens and classnames. The correct classnames for column-gap would be using the formula (x / 4) * 100 as describes in the above comment. Please take reference from the border classes

Comment on lines 27 to 48
{
className: 'white-space-nowrap',
properties: 'white-space: nowrap;',
},
{
className: 'bottom-0',
properties: 'bottom: 0;',
},
// Visibility utilities
{
className: 'visible',
properties: 'visibility: visible;',
},
{
className: 'invisible',
properties: 'visibility: hidden;',
},
// User select utilities
{
className: 'user-select-none',
properties: 'user-select: none;',
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please create a separate table for each group of class for better user discoverability and avoid pagination in tables to display different groups of classes

Comment on lines 300 to 320
Try to <span className="font-weight-bold">select this text</span> - it cannot be selected
</div>
</Card>
</div>
<div className="w-25 mb-3 user-select-all">
<Card className="p-4 bg-secondary-lightest shadow-s">
<Heading appearance="subtle" size="s" className="color-accent3-dark mb-2">
user-select-all
</Heading>
<div className="bg-white p-3 rounded-10 border border-accent3">
Click anywhere to <span className="font-weight-bold">select all of this text</span> at once
</div>
</Card>
</div>
<div className="w-25 mb-3 user-select-text">
<Card className="p-4 bg-secondary-lightest shadow-s">
<Heading appearance="subtle" size="s" className="color-success-dark mb-2">
user-select-text
</Heading>
<div className="bg-white p-3 rounded-10 border border-success">
This has <span className="font-weight-bold">normal text selection</span> behavior (default)
Copy link
Collaborator

Choose a reason for hiding this comment

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

font-weight-bold class does not exist in our design system. Please remove it.

</Heading>
<div className="d-flex flex-wrap gap-4">
<div className="w-25 mb-5">
<div className="text-center mb-2 color-primary-dark font-weight-bold">word-break-normal</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

classname text-center does not exist in our design system. Please remove it.

</Heading>
<div className="d-flex flex-wrap gap-4">
<div className="w-25 mb-5">
<div className="text-center mb-2 color-primary-dark font-weight-bold">word-break-normal</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

classname font-weight-bold does not exist in our design system. Please remove it from everywhere.

Comment on lines 349 to 393
<div className="text-center mb-2 color-accent1-dark font-weight-bold">word-break-all</div>
<Card className="p-3 bg-secondary-lightest position-relative vh-25 shadow-s">
<div className="position-absolute bottom-0 right-0 bg-warning color-white p-1 px-2 font-size-xs rounded-bottom-left-1 z-index-1">
break-all
</div>
<div className="word-break-all border border-dashed border-accent1 p-2 pt-3 h-50 bg-white rounded-10">
Break-all: ThisIsALongWordThatWillBreakAtAnyCharacter
</div>
<Text appearance="subtle" size="small" className="mt-2 color-accent1-dark">
Breaks can occur between any two characters
</Text>
</Card>
</div>

<div className="w-25 mb-5">
<div className="text-center mb-2 color-success-dark font-weight-bold">word-break-word</div>
<Card className="p-3 bg-secondary-lightest position-relative vh-25 shadow-s">
<div className="position-absolute bottom-0 right-0 bg-success color-white p-1 px-2 font-size-xs rounded-bottom-left-1 z-index-1">
break-word
</div>
<div className="word-break-word border border-dashed border-success p-2 pt-3 h-50 bg-white rounded-10">
Break-word: ThisIsALongWordThatWillBreakAtAppropriatePoints
</div>
<Text appearance="subtle" size="small" className="mt-2 color-success-dark">
Breaks at word boundaries when possible
</Text>
</Card>
</div>

<div className="w-25 mb-5">
<div className="text-center mb-2 color-accent2-dark font-weight-bold">word-wrap-break</div>
<Card className="p-3 bg-secondary-lightest position-relative vh-25 shadow-s">
<div className="position-absolute bottom-0 right-0 bg-danger color-white p-1 px-2 font-size-xs rounded-bottom-left-1 z-index-1">
break-word
</div>
<div className="word-wrap-break border border-dashed border-accent2 p-2 pt-3 h-50 bg-white rounded-10">
Word-wrap: ThisIsALongWordThatBreaksBeforeOverflowing
</div>
<Text appearance="subtle" size="small" className="mt-2 color-accent2-dark">
Also known as overflow-wrap in modern CSS
</Text>
</Card>
</div>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove classes text-center and font-weight-bold from everywhere as these classes does not exist.

Comment on lines 402 to 436
<div className="text-center mb-2 color-primary-dark font-weight-bold">object-fit-cover</div>
<Card className="vh-25 overflow-hidden shadow-s bg-secondary-lightest p-2">
<img
src="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMzAwIiBoZWlnaHQ9IjIwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KIDxnPgogIDxyZWN0IHdpZHRoPSIzMDAiIGhlaWdodD0iMjAwIiBmaWxsPSIjZGRkZGRkIi8+CiAgPHJlY3QgeD0iNTAiIHk9IjUwIiB3aWR0aD0iMjAwIiBoZWlnaHQ9IjEwMCIgZmlsbD0iIzU1NTU1NSIvPgogIDx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBmb250LWZhbWlseT0iQXJpYWwiIGZvbnQtc2l6ZT0iMThweCIgZmlsbD0id2hpdGUiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGRvbWluYW50LWJhc2VsaW5lPSJtaWRkbGUiPkV4YW1wbGUgSW1hZ2U8L3RleHQ+CiA8L2c+Cjwvc3ZnPg=="
alt="Example"
className="object-fit-cover w-100 h-100 rounded-10 border border-primary"
/>
</Card>
<Text appearance="subtle" size="small" className="text-center mt-2 color-primary-dark">
Covers entire area, maintaining aspect ratio
</Text>
</div>
<div className="w-25 mb-3">
<div className="text-center mb-2 color-accent3-dark font-weight-bold">object-fit-contain</div>
<Card className="vh-25 overflow-hidden shadow-s bg-secondary-lightest p-2">
<img
src="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMzAwIiBoZWlnaHQ9IjIwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KIDxnPgogIDxyZWN0IHdpZHRoPSIzMDAiIGhlaWdodD0iMjAwIiBmaWxsPSIjZGRkZGRkIi8+CiAgPHJlY3QgeD0iNTAiIHk9IjUwIiB3aWR0aD0iMjAwIiBoZWlnaHQ9IjEwMCIgZmlsbD0iIzU1NTU1NSIvPgogIDx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBmb250LWZhbWlseT0iQXJpYWwiIGZvbnQtc2l6ZT0iMThweCIgZmlsbD0id2hpdGUiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGRvbWluYW50LWJhc2VsaW5lPSJtaWRkbGUiPkV4YW1wbGUgSW1hZ2U8L3RleHQ+CiA8L2c+Cjwvc3ZnPg=="
alt="Example"
className="object-fit-contain w-100 h-100 rounded-10 border border-accent3"
/>
</Card>
<Text appearance="subtle" size="small" className="text-center mt-2 color-accent3-dark">
Fits within the box without cropping
</Text>
</div>
<div className="w-25 mb-3">
<div className="text-center mb-2 color-success-dark font-weight-bold">object-fit-fill</div>
<Card className="vh-25 overflow-hidden shadow-s bg-secondary-lightest p-2">
<img
src="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMzAwIiBoZWlnaHQ9IjIwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KIDxnPgogIDxyZWN0IHdpZHRoPSIzMDAiIGhlaWdodD0iMjAwIiBmaWxsPSIjZGRkZGRkIi8+CiAgPHJlY3QgeD0iNTAiIHk9IjUwIiB3aWR0aD0iMjAwIiBoZWlnaHQ9IjEwMCIgZmlsbD0iIzU1NTU1NSIvPgogIDx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBmb250LWZhbWlseT0iQXJpYWwiIGZvbnQtc2l6ZT0iMThweCIgZmlsbD0id2hpdGUiIHRleHQtYW5jaG9yPSJtaWRkbGUiIGRvbWluYW50LWJhc2VsaW5lPSJtaWRkbGUiPkV4YW1wbGUgSW1hZ2U8L3RleHQ+CiA8L2c+Cjwvc3ZnPg=="
alt="Example"
className="object-fit-fill w-100 h-100 rounded-10 border border-success"
/>
</Card>
<Text appearance="subtle" size="small" className="text-center mt-2 color-success-dark">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use some better example to clearly define the difference between these classes. Right now all of them looks similar
image

Comment on lines 497 to 511
<Card className="p-4 bg-secondary-lightest shadow-s">
<Heading appearance="subtle" size="s" className="color-primary-dark mb-2">
list-style-disc
</Heading>
<div className="bg-white p-3 rounded-10 border border-primary">
<ul className="list-style-disc ml-4">
<li>Item 1</li>
<li>Item 2</li>
<li>Item 3</li>
</ul>
</div>
</Card>
</div>
<div className="w-25 mb-3">
<Card className="p-4 bg-secondary-lightest shadow-s">
Copy link
Collaborator

Choose a reason for hiding this comment

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

shadow-s as a classname does not exist in our design system. please remove it from everywhere.

Copy link
Collaborator

@anuradha9712 anuradha9712 left a comment

Choose a reason for hiding this comment

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

We need to add more classes for background. It should consist of all the colors defined in our DS.

Comment on lines 41 to 47
.visible {
visibility: visible !important;
}

.invisible {
visibility: hidden !important;
}
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Please change this classname to something else like:

  • visibility-0 & visibility-1
  • visibility-hidden & visibility-show

Comment on lines +102 to +114
<div className="overflow-auto p-4 bg-secondary-lightest border rounded-05 h-100">{longText}</div>
<Text className="mt-3">Scrollbars appear only when necessary</Text>
</div>
</div>

{/* overflow-hidden */}
<div className="mr-8 mb-6 w-25">
<Text weight="strong" className="mb-3">
overflow-hidden
</Text>
<div className="position-relative">
<div className="overflow-hidden p-4 bg-secondary-lightest border rounded-05 h-100">{longText}</div>
<Text className="mt-3">Content is clipped, no scrollbars</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use some better example, right now all the examples looks similar
image

Comment on lines +263 to +267
<Text className="mt-3 mb-5">A real-world example of using overflow utilities in a UI component:</Text>

<div className="d-flex flex-wrap">
<div className="mr-8 mb-6 w-25">
<Text weight="strong" className="mb-3">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add some spacing in this example
image

Comment on lines +29 to +43
const placementData = [
{
className: 'top-0, top-25, top-50, top-75, top-100, top-auto',
properties:
'top: 0 !important;\ntop: 25% !important;\ntop: 50% !important;\ntop: 75% !important;\ntop: 100% !important;\ntop: auto !important;',
},
{
className: 'right-0, right-25, right-50, right-75, right-100, right-auto',
properties:
'right: 0 !important;\nright: 25% !important;\nright: 50% !important;\nright: 75% !important;\nright: 100% !important;\nright: auto !important;',
},
{
className: 'bottom-0, bottom-25, bottom-50, bottom-75, bottom-100, bottom-auto',
properties:
'bottom: 0 !important;\nbottom: 25% !important;\nbottom: 50% !important;\nbottom: 75% !important;\nbottom: 100% !important;\nbottom: auto !important;',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This example is not scrollable as its mentioned here
image

{
className: 'position-fixed',
properties: 'position: fixed ;',
properties: 'position: fixed !important;',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add example for position: fixed as well
image

Comment on lines +29 to +38
const placementData = [
{
className: 'top-0, top-25, top-50, top-75, top-100, top-auto',
properties:
'top: 0 !important;\ntop: 25% !important;\ntop: 50% !important;\ntop: 75% !important;\ntop: 100% !important;\ntop: auto !important;',
},
{
className: 'right-0, right-25, right-50, right-75, right-100, right-auto',
properties:
'right: 0 !important;\nright: 25% !important;\nright: 50% !important;\nright: 75% !important;\nright: 100% !important;\nright: auto !important;',
Copy link
Collaborator

@anuradha9712 anuradha9712 May 10, 2025

Choose a reason for hiding this comment

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

Please write all the properties more clearly as it is not readable right now
image

Comment on lines +67 to +82
const zIndexData = [
{
className: 'z-index-n1, z-index-0, z-index-1',
properties: 'z-index: -1 !important;\nz-index: 0 !important;\nz-index: 1 !important;',
},
{
className: 'z-index-2, z-index-3, z-index-4, z-index-5',
properties: 'z-index: 2 !important;\nz-index: 3 !important;\nz-index: 4 !important;\nz-index: 5 !important;',
},
{
className: 'z-index-10, z-index-50, z-index-100, z-index-1000, z-index-auto',
properties:
'z-index: 10 !important;\nz-index: 50 !important;\nz-index: 100 !important;\nz-index: 1000 !important;\nz-index: auto !important;',
},
];

Copy link
Collaborator

Choose a reason for hiding this comment

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

Write all these classes in different rows for better readability
image

@anuradha9712
Copy link
Collaborator

anuradha9712 commented May 10, 2025

Please follow some basic but important points to check before submitting any PR for review:

  • Review your own code thoroughly — Don’t rely on the reviewer to catch issues you could spot yourself.
  • Check if the code does not contains any un-used variables or classnames which decreases the code quality
  • Verify that all the examples are readable and work as expected
  • Verify that the code aligns with our design guidelines, semantic HTML standards, and CSS best practices.
  • Discuss your design decisions with your team before implementing

Following these simple practices helps streamline the review process and saves time for both the reviewer and the contributor.

Happy coding :)

@anuradha9712
Copy link
Collaborator

@codex review this PR

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veekays veekays force-pushed the feat-utility-classes branch from 6d42183 to 8dd4928 Compare December 14, 2025 20:03
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.

2 participants