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

fix(labware-library): fix labware-library css issue #16502

Merged
merged 5 commits into from
Oct 17, 2024

Conversation

koji
Copy link
Contributor

@koji koji commented Oct 16, 2024

Overview

fix labware-library css issue

the issue we are facing came from the following PR.
vitejs/vite#16051

I think ideal fix is to align our css with the PR or convert all css into styled-components, but the thing is that at this moment we don't have time to do that.

other solution would be use vite v5.1.6 since the change is in v5.2.0.

before

Screenshot 2024-10-16 at 5 36 02 PM

after

Screenshot 2024-10-16 at 5 35 47 PM

Test Plan and Hands on Testing

go to and check that the layout is not broken.
http://sandbox.labware.opentrons.com/fix_labware-library-css-bundle-issue/

Changelog

Review requests

Risk assessment

low

fix labware-library css issue

close
@koji koji requested review from shlokamin and jerader October 16, 2024 21:18
@koji koji marked this pull request as ready for review October 16, 2024 21:29
@koji koji requested a review from a team as a code owner October 16, 2024 21:29
@koji koji removed the request for review from a team October 16, 2024 21:29
Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

wow wow wow. tree shaking and bundle optimizations are sooooooooo cool until they aren't. @sfoster1 you'll probably get a kick out of this

thank you koji! lets just add some comments and add TS annotations.

@@ -0,0 +1,13 @@
export const cssModuleSideEffect = (): any => {
Copy link
Member

Choose a reason for hiding this comment

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

can we add some comments explaining what we're doing and why, and also maybe links to the github issue for vite? coming back later and understanding why we're doing this will be important. im sure this wont be the last time we have to deal with this 😢

Copy link
Member

Choose a reason for hiding this comment

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

can we also add the appropriate TS types to this plugin?

Copy link
Member

Choose a reason for hiding this comment

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

+1 to wanting comments with a link to the vite pr

@koji koji merged commit 703e01d into edge Oct 17, 2024
14 checks passed
@koji koji deleted the fix_labware-library-css-bundle-issue branch October 17, 2024 22:33
TamarZanzouri pushed a commit that referenced this pull request Oct 18, 2024
* fix(labware-library): fix labware-library css issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants