Remove defaultProps to support React 18.3.0 #1650
Conversation
The `defaultProps` has been deprecated for some time for function components. I have gone through and updated all function components to use Default Parameters instead. You can see the deprecation notice at: https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#deprecate-defaultprops-on-function-components
Default props have been removed from functional components, but not necessarily from styled components. Since the change I made changed the chromatic tests, I have now reverted this single defaultProp.
|
@penx, Could you have a look and see what you think? I've removed all the |
|
2 minor bits of feedback but all good otherwise:
Thanks for the contribution! |
|
The fixed width font has changed in some stories on Chromatic, I suspect due to differences elsewhere or within Chromatic but worth checking against the main branch to be certain. |
I'm not sure what I'm meant to be looking at. I've compared Chromatic to main (GitHub Pages) and my branch (local and Chromatic), and it seems consistent (apart from Chromatic not having the intros and args). |
|
besides the font differences, Chromatic is also showing what looks like a margin difference on a radio button story. this PR seems to have a significantly reduced margin |
|
Interesting, I have not changed any of the stylings. 😕 |
@stevesims I think you're referring to this, which I think is also the fixed width thing, there seems to be a @richrace If these stories look the same in main and this branch then I think we're good, likely due to a change with chromatic's rendering since we last approved something. The thing to look out for is the font family and size being used for the fixed width parts of the stories above. Are you able to open the Chromatic links and see the visual diffs? |
|
Ah the "UI Tests" I can see what you mean now. I'll investigate. |
|
The Code is the same, Bold is the same, Italics are the same, So it's:
|
|
Are you happy for me to Accept the visual changes that https://www.chromatic.com/build?appId=5ad61aab7f00090020dfdba8&number=2659 have thrown up? |
Sure if you have access to accept? Accept the ones that are due to a difference in rendering fixed width fonts, which I think is all of them |
|
That's all done. Just the cypress tests to run and it should be good to merge and be released. |
yes - that was the thing. if you look at the "1 up" view then there's a green line shown right at the bottom having taken a closer look directly checking out the two respective storybooks for each branch being compared, I can see no difference - they are the same height this therefore seems to be another odd chromatic rendering issue, like the minor font rendering differences the other stories are showing - so it feels safe to ignore. 😀 |
|
On my fork, the Cypress tests run (and pass), but they don't seem to get run on the PR. What am I missing here? Edit: See the commit below b799832 |
|
This should be all good to go. |
|
@richrace thanks! Do you want to attempt a release and see how far you get? https://github.com/govuk-react/govuk-react/blob/main/CONTRIBUTING.md#releasing |
See this release from React: https://github.com/facebook/react/releases/tag/v18.3.0