feat: rebuild component-library using Vite#866
Conversation
michael-odonovan
changed the title
|
Yeha, I saw seeing issues with icons too, think that 'mock.asset' thing is likely the cause; don't fully understand what that is or how its supposed to be used yet, but will pick over it next week. Review list so far, just in case it's useful, will add to this later:
|
|
Along with the issues mentioned above, I noticed the below one: The LogoLinked component seems to be missing under the Molecules section in the localhost build. I can see it in the current master envt here: https://cr-component-library.netlify.app/#logolinked, but it doesn’t appear in this branch. |
|
@michael-odonovan tested the fixes, all working as expected. Just a reminder of what Andy mentioned above on
But it can be done in another PR. Great work on this, looking really good. |
Absolutely; this is a mammoth bit of work with a lot of nonsense to deal with and you've done us all proud. Will just sense-check the updated diffs, then suss out that |
| width: 24px; | ||
| height: 24px; | ||
| background-image: url(${({ $icon }) => $icon}); | ||
| background-image: url("${({ $icon }) => $icon}"); |
There was a problem hiding this comment.
Ohhhh, that's interesting; nicely sussed - I wonder if that due to the react or the styled-components update, or it's a Vitey thing?
There was a problem hiding this comment.
I really don't know why it passed previously - maybe to do with React update being stricter.
|
|
||
| expect(tree).toMatchInlineSnapshot(` | ||
| .c0 { | ||
| display: -webkit-inline-box; |
There was a problem hiding this comment.
We've still got autoprefixer in the package.json but let's just leave it in for sanity and figure out the config in the next PR
| @@ -175,8 +171,6 @@ exports[`renders Picture with flat props correctly 1`] = ` | |||
|
|
|||
| <div | |||
| className="c0 lazyload" | |||
| height="100%" | |||
There was a problem hiding this comment.
Still not fully sure about these changes to Picture (no doubt due to the Transient Props Tango), but given that Krupa's already eyeballed everything without flagging, I guess we can leave it for now and revisit if needs be.
There was a problem hiding this comment.
Guess we should fix this, the image on PictureOrVideo is getting cut off compared to the master one
Master
…brary into 4973_CL_rebuild
|
@michael-odonovan Few minor issues that can go in another PR.
Master 
Master 
New build: 
Same issue on Master too 
New build: 
Master 
The copy on single message on the new build is stretched but on master is formatted in the center 
Master 
|
|
@michael-odonovan @AndyEPhipps Issues 1, 2, 4 and 9 are specific to the new build, other issues are on both new and master. All minor ones which I thought may not affect but flagging just in case. |
…er wrapping The commit 3a1c727 wrapped component examples in ExampleContainer, changing the DOM structure. Test selectors using nth-child now fail because labels are no longer direct children of the preview div. Updates: - Checkbox: Use page.locator with .nth() to select specific labels instead of nth-child selectors that don't match the new nested structure - ImpactSlider: Fix selectors that assumed linear DOM structure These changes make selectors more robust and work with the new layout. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Destructure data-testid from rest props so it only goes to the Label element, not to StyledCheckboxInput. This prevents the same attribute from appearing on multiple elements, which causes Playwright strict mode errors. Change: - Extract data-testid before spreading rest to StyledCheckboxInput - Only StyledCheckboxInput gets the remaining inputProps (without data-testid) - Label gets data-testid attribute exclusively This makes selectors unambiguous: label[data-testid="..."] now resolves to exactly 1 element instead of 2. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Scope both impact-slider-amount-text AND impact-slider--moneybuy selectors with their parent ImpactSlider-example to avoid strict mode violations. Changes: - All assertions in test 1: scope to [data-testid="ImpactSlider-example-1"] - All assertions in test 2: scope to [data-testid="ImpactSlider-example-2"] This ensures when multiple ImpactSlider instances exist on the page, each selector targets only the elements within the intended example. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
🎉 This PR is included in version 8.74.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
PR description
What is it doing?
Rebuild using Vite and native React/Javascript functionality.
Moves this repo off dependency on an unmaintained package and updates to React 18.
Why is this required?
Maintenance / Security updates.
link to Jira ticket:
https://comicrelief.atlassian.net/browse/ENG-4973
Quick Checklist:
My PR title follows the Conventional Commit spec.
I have filled out the PR description as per the template above.
I have added tests to cover new or changed behaviour.
I have updated any relevant documentation.
Important! - lastly, make sure to squash merge...