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

Add disabled groups #1764

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Add disabled groups #1764

merged 1 commit into from
Dec 2, 2024

Conversation

mollerjorge
Copy link
Collaborator

What does this implement/fix?

This PR adds support for showing groups that are disabled (filtered out) in the RFM grid with a greyed out background and a dashed border.

Storybook link

https://dhvi79nyj9utsllw.tunnel.shopifycloud.tech/?path=/story/polaris-viz-charts-grid-playground--missing-groups

Screenshot 2024-11-21 at 8 19 53 PM

Before merging

  • Check your changes on a variety of browsers and devices.

  • Update the Changelog's Unreleased section with your changes.

  • Update relevant documentation, tests, and Storybook.

  • Make sure you're exporting any new shared Components, Types and Utilities from the top level index file of the package

@mollerjorge mollerjorge self-assigned this Nov 21, 2024
@mollerjorge mollerjorge force-pushed the jorge/grid-grey-out-groups branch from 86d5390 to 208ddad Compare November 21, 2024 23:22
Copy link

github-actions bot commented Nov 21, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
polaris-viz-core-cjs 61.64 KB (0%) 1.3 s (0%) 981 ms (-21.46% 🔽) 2.3 s
polaris-viz-cjs 223.76 KB (+0.07% 🔺) 4.5 s (+0.07% 🔺) 1.6 s (-13.21% 🔽) 6.1 s
polaris-viz-esm 181.02 KB (+0.07% 🔺) 3.7 s (+0.07% 🔺) 1.7 s (+11.58% 🔺) 5.3 s
polaris-viz-css 5.56 KB (+0.18% 🔺) 112 ms (+0.18% 🔺) 379 ms (+50.17% 🔺) 491 ms
polaris-viz-esnext 187.79 KB (+0.08% 🔺) 3.8 s (+0.08% 🔺) 1.5 s (-7.95% 🔽) 5.2 s

@mollerjorge mollerjorge force-pushed the jorge/grid-grey-out-groups branch from 208ddad to 60a9356 Compare November 26, 2024 12:37
Comment on lines +26 to +51
<g>
<rect
x={x + BACKGROUND_GAP / 2}
y={y + BACKGROUND_GAP / 2}
width={adjustedWidth}
height={adjustedHeight}
fill={fill}
rx="4"
ry="4"
opacity={opacity}
/>
{isDisabled && (
<rect
x={x + BACKGROUND_GAP / 2 + 1}
y={y + BACKGROUND_GAP / 2 + 1}
width={adjustedWidth - 2}
height={adjustedHeight - 2}
fill="none"
stroke="#E3E3E3"
strokeWidth="1"
strokeDasharray="4 4"
rx="3"
ry="3"
/>
)}
</g>
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity, is needed to return both <rect /> even when isDisabled is true, or can we just return the disabled style <rect /> only when this is true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good question! so the inner <rect simulates an inner dashed border for the disabled groups. I tried to add the strokeDashArray on the parent rect, but the dashed border doesn't appear inside, instead appears outside, and in the designs the dashed borders are inside the rectangle.

It basically simulates a container for an inner border when the group is disabled.

@mollerjorge mollerjorge force-pushed the jorge/grid-grey-out-groups branch from 60a9356 to 4ead2cd Compare November 27, 2024 14:05
@mollerjorge mollerjorge force-pushed the jorge/grid-grey-out-groups branch from 4ead2cd to beccc58 Compare November 27, 2024 22:03
@mollerjorge mollerjorge merged commit 725784d into main Dec 2, 2024
6 checks passed
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