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.

Sign up for GitHub

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

fix(ValueCard) Remove XSMALL from LEGACY_CARD_SIZES because we are using and displaying unwanted warnings #3935

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open

fix(ValueCard) Remove XSMALL from LEGACY_CARD_SIZES because we are using and displaying unwanted warnings #3935

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
167c560
Remove XSMALL from LEGACY_CARD_SIZES because we are using and display…
anishkumar-bhut Mar 7, 2025
d537a99
document corrected
anishkumar-bhut Mar 7, 2025
e00df1d
Merge branch 'next' into anish-valuecard-size-warning
anishkumar-bhut Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react/src/components/ValueCard/ValueCard.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Optionally you may use a callback as the cardVariables value that will be given
| renderExpandIcon | function, object | | Define the icon render to be rendered. Can be a React component class |
| hideHeader | bool | | should hide the header |
| showOverflow | deprecate(<br/> PropTypes.bool,<br/> '\nThe prop `showOverflow` for Card has been deprecated. It was previously needed for a custom positioned tooltip in the ValueCard, but the ValueCard now uses the default positioning of the tooltip. The `iot--card--wrapper--overflowing` class has been removed. For automated testing, you can target `data-testid="Card"` instead.'<br/>) | | sets the CardWrapper CSS overflow to visible |
| size | enum: <br/>'SMALL'<br/>'SMALLWIDE'<br/>'SMALLFULL'<br/>'MEDIUMTHIN'<br/>'MEDIUM' <br/> 'MEDIUMWIDE' <br/> 'LARGETHIN' <br/> 'LARGE' <br/> 'LARGEWIDE' | | card size |
| size | enum: <br/>'XSMALL'<br/>'SMALL'<br/>'SMALLWIDE'<br/>'SMALLFULL'<br/>'MEDIUMTHIN'<br/>'MEDIUM' <br/> 'MEDIUMWIDE' <br/> 'LARGETHIN' <br/> 'LARGE' <br/> 'LARGEWIDE' | | card size |
| layout | enum: <br/>'VERTICAL'<br/>'HORIZONTAL'<br/> | | |
| breakpoint | enum: <br/>'lg'<br/>'max'<br/>'md'<br/>'sm'<br/>'xl'<br/>'xs'<br/> | | |
| availableActions | shape | { edit: false, clone: false, delete: false, range: false, expand: false,} | |
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/constants/LayoutConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const COLORS = {
};

export const CARD_SIZES = {
XSMALL: 'XSMALL',
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this supported ? As far as I'm aware support for XSMALL was removed. and when ever someone use XSMALL we used to change it to 'SMALL'

This may have impact on monitor dashboards.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@herleraja Hello. Support for xsmall was removed, but we have a scenario in Maximo where the use of it is required again. It will only be allowed for certain simple Value cards, and there is likely some CSS cleanup and other work that will need to go in to that card type. What would work for Monitor here? Is there a condition or check we could add to isolate you from this change? @anishkumar-bhut @hectordavis @stuckless

Copy link
Collaborator

Choose a reason for hiding this comment

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

Where exactly is the warning coming from? From what I can see here, ValueCard still uses the LEGACY_CARD_SIZES as expected sizes

Copy link
Collaborator

@herleraja herleraja Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@JordanWSmith15 Just checked. I think the monitor should not be affected by this change.

We override the props accordingly.

image

Also, this may need to be well tested, because its used directly or indirectly
example if the size is small we won't display the error message icons. so we need to add the XSMALL size here too.
image

Copy link
Collaborator

@herleraja herleraja Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@hectordavis the warning thrown in getUpdatedCardSize but that's thrown only in development mode. It should not effect in production.
image

SMALL: 'SMALL',
SMALLWIDE: 'SMALLWIDE',
SMALLFULL: 'SMALLFULL',
Expand All @@ -24,7 +25,6 @@ export const CARD_SIZES = {
};

export const LEGACY_CARD_SIZES = {
XSMALL: 'XSMALL',
XSMALLWIDE: 'XSMALLWIDE',
TALL: 'TALL',
WIDE: 'WIDE',
Expand Down
4 changes: 1 addition & 3 deletions packages/react/src/utils/cardUtilityFunctions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ export const compareGrains = (grain1, grain2) => {

export const getUpdatedCardSize = (oldSize) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If its needed for value card may be you can do it specifically for value card. looks like getUpdatedCardSize function getting used in almost all the cards
image

const changedSize =
oldSize === 'XSMALL'
? 'SMALL'
: oldSize === 'XSMALLWIDE'
oldSize === 'XSMALLWIDE'
? 'SMALLWIDE'
: oldSize === 'WIDE'
? 'MEDIUMWIDE'
Expand Down