feat: use React.forwardRef in all components #4233

layershifter · 2021-07-23T12:47:41Z

This PR is a part of work for Semantic UI React v3 🎉

All components in this PR have been converted to be functional components (Dropdown is an exception, it uses a wrapper) and use React.forwardRef() to forward refs natively.

github-actions · 2021-07-23T12:49:50Z

size-limit report

Path	Size
bundle-size/dist/Button.size.js	54.04 KB (-8.79% 🔽)
bundle-size/dist/Icon.size.js	27.35 KB (+7.85% 🔺)
bundle-size/dist/Image.size.js	49.6 KB (-8.74% 🔽)
bundle-size/dist/Modal.size.js	62.6 KB (-10.12% 🔽)
bundle-size/dist/Portal.size.js	37.17 KB (-9.29% 🔽)

codecov-commenter · 2021-07-23T12:56:55Z

Codecov Report

Base: 99.75% // Head: 99.51% // Decreases project coverage by -0.23% ⚠️

Coverage data is based on head (88974b5) compared to base (80c4d82).
Patch coverage: 99.05% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4233      +/-   ##
==========================================
- Coverage   99.75%   99.51%   -0.24%     
==========================================
  Files         180      186       +6     
  Lines        3248     3511     +263     
==========================================
+ Hits         3240     3494     +254     
- Misses          8       17       +9

Impacted Files	Coverage Δ
src/lib/hooks/useClassNamesOnNode.js	`100.00% <ø> (ø)`
src/modules/Popup/lib/createReferenceProxy.js	`57.14% <ø> (ø)`
src/modules/Search/SearchCategoryLayout.js	`100.00% <ø> (ø)`
src/lib/hooks/useEventCallback.js	`75.00% <75.00%> (ø)`
src/modules/Dimmer/Dimmer.js	`90.47% <88.23%> (-9.53%)`	⬇️
src/lib/hooks/useAutoControlledValue.js	`91.66% <91.66%> (ø)`
src/modules/Sticky/Sticky.js	`94.64% <94.54%> (-4.44%)`	⬇️
src/modules/Modal/Modal.js	`98.83% <98.52%> (-1.17%)`	⬇️
src/modules/Popup/Popup.js	`98.94% <98.80%> (+0.01%)`	⬆️
src/addons/Confirm/Confirm.js	`100.00% <100.00%> (ø)`
... and 165 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

* chore(Ref): remove component * fix tests

* chore(Search): use React.forwardRef() * add test for ref forwarding * fix tests, add comments

* chore: refactor tests for shorthands to use mount() * add comment * fix tests with unmount

* Convert form to function component * Add ref forwarding to Form component * Update test/specs/collections/Form/Form-test.js Co-authored-by: Oleksandr Fediashov <[email protected]> Co-authored-by: Oleksandr Fediashov <[email protected]>

* Add ref to FormButton * Add ref to FormCheckbox * Add forwardRef to FormGroup * Remove accidental `only` calls

* Add ref to form field component * Move ref to controlProps * Add more forwardsRef tests * Remove unnecessary describe wrapper * update tests Co-authored-by: Oleksandr Fediashov <[email protected]>

* chore(Select): use React.forwardRef() * chore(FormSelect): use React.forwardRef() * chore(FormDropdown): use React.forwardRef() * fix tests, fix Dropdown component Co-authored-by: Oleksandr Fediashov <[email protected]>

sunech · 2022-12-13T12:46:40Z

Awesome guys, really nice work with the progress these last couple of days! 👏👏

felixmosh · 2022-12-13T13:03:25Z

#4408

PR is merged, thanks 🎉 It was the last step before the release. I will ping @levithomason to trigger 3.0.0-beta.

Do you think that we can use it for the displayName task?

Yes, thanks for doing that. I created an issue (#4409) to clarify the plan and steps related to release. Feel free to create a PR once this gets merged 👍

What should be merged? :]

PerryRylance · 2022-12-13T13:32:16Z

Thank you folks, been waiting a while for this

layershifter · 2022-12-14T08:40:41Z

What should be merged? :]

Yes, thanks for doing that. I created an issue (#4409) to clarify the plan and steps related to release. Feel free to create a PR once this PR gets merged 👍

Oops, missed a word.

levithomason · 2023-02-28T16:06:05Z

Giving this a test and look over 👀 Fantastic effort everybody, thank you 😍

levithomason · 2023-02-28T17:04:33Z

I have been testing components and looking over changes this morning. Looks good! I will release the beta now and we can follow up on any clean up items. Superb job as usual @layershifter, much appreciated.

layershifter · 2023-02-28T17:38:59Z

Thanks everyone 👍

3.0.0-beta.0 is out 🎉 Please try and report issues 🙏

felixmosh · 2023-02-28T17:47:12Z

except React.ref, what are the changed?

layershifter · 2023-02-28T17:56:29Z

@felixmosh I will publish migration notes and guidance in upcoming days, shortly:

refs forwarding everywhere
BREAKING CHANGE remove Ref component chore(Ref): remove component #4286
BREAKING CHANGE remove Visibility component chore(Visibility): remove component #4257
BREAKING CHANGE Input, Button & TextArea are forwarding refs to native elements

sunech · 2023-03-04T15:19:59Z

Been running 3.0.0-beta.0 for in my dev environment for some days now. Tested a lot of the pages and functionality with it.

No issues so far, apart from the expected refs that needed to be changed for form elements. Great work!

layershifter · 2023-07-09T10:45:04Z

FYI Migration guide is available (https://react.semantic-ui.com/migration-guide/), changelog was also updated.

dominikdosoudil · 2024-11-12T08:59:13Z

src/modules/Popup/Popup.js

@@ -195,7 +251,7 @@ export default class Popup extends Component {
        ) : (
          children
        )}
-        {hideOnScroll && <EventStack on={this.hideOnScroll} name='scroll' target='window' />}
+        {hideOnScroll && <EventStack on={hideOnScroll} name='scroll' target='window' />}


@layershifter after refactoring the Popup to FC, there is a hideOnScroll callback and prop name clash. It causes the guard be always truthy and thus the prop does not work. The callback should be renamed.

I think that this PR should be reviewed again because there might be more occurrences of this same issue. What do you think?

@dominikdosoudil good catch, please trigger an issue and feel free to submit a PR 👍

Apparently there exists both issue and PR #4490 (at least for the popup)

layershifter added the PR: Internal 🏠 label Jul 23, 2021

layershifter mentioned this pull request Jul 23, 2021

Use forward refs in Components to access ref instances #3819

Closed

layershifter changed the title ~~chore: remove .state() assertions in tests (#4232)~~ chore: v3 Jul 23, 2021

layershifter changed the title ~~chore: v3~~ chore: Semantic UI React v3 Jul 23, 2021

vercel bot deployed to Preview July 23, 2021 13:48 View deployment

layershifter mentioned this pull request Jul 26, 2021

findDOMNode is deprecated in StrictMode < Button / > #4050

Closed

vercel bot deployed to Preview July 26, 2021 08:32 View deployment

vercel bot deployed to Preview July 26, 2021 08:43 View deployment

vercel bot deployed to Preview July 26, 2021 09:19 View deployment

vercel bot deployed to Preview July 26, 2021 10:20 View deployment

vercel bot deployed to Preview July 26, 2021 10:35 View deployment

vercel bot deployed to Preview July 26, 2021 10:59 View deployment

vercel bot deployed to Preview July 26, 2021 12:28 View deployment

vercel bot deployed to Preview July 26, 2021 12:31 View deployment

vercel bot deployed to Preview July 26, 2021 12:47 View deployment

vercel bot deployed to Preview July 26, 2021 12:55 View deployment

vercel bot deployed to Preview July 26, 2021 13:15 View deployment

vercel bot deployed to Preview July 26, 2021 13:42 View deployment

vercel bot deployed to Preview July 26, 2021 14:14 View deployment

vercel bot deployed to Preview July 26, 2021 14:24 View deployment

vercel bot deployed to Preview July 26, 2021 14:46 View deployment

vercel bot deployed to Preview July 26, 2021 15:38 View deployment

vercel bot deployed to Preview July 27, 2021 10:13 View deployment

vercel bot deployed to Preview July 27, 2021 10:54 View deployment

vercel bot deployed to Preview July 27, 2021 11:17 View deployment

layershifter linked an issue Jul 27, 2021 that may be closed by this pull request

Use forward refs in Components to access ref instances #3819

Closed

vercel bot deployed to Preview July 27, 2021 12:04 View deployment

vercel bot deployed to Preview July 28, 2021 08:51 View deployment

layershifter and others added 14 commits December 13, 2022 13:44

chore(Ref): remove component (#4286)

5f8b882

* chore(Ref): remove component * fix tests

chore(Search): use React.forwardRef() (#4330)

6396cf2

* chore(Search): use React.forwardRef() * add test for ref forwarding * fix tests, add comments

chore: refactor Modal tests (#4339)

23c904f

chore: remove unused test utils (#4340)

fb8c9e5

chore: refactor tests for classnames to use mount() (#4341)

ca83470

chore: refactor tests for shorthands to use mount() (#4342)

5c1d808

* chore: refactor tests for shorthands to use mount() * add comment * fix tests with unmount

fix(Dropdown): use ref forwarding (#4338)

e331591

Add forward ref to Form (#4364)

10a1633

* Convert form to function component * Add ref forwarding to Form component * Update test/specs/collections/Form/Form-test.js Co-authored-by: Oleksandr Fediashov <[email protected]> Co-authored-by: Oleksandr Fediashov <[email protected]>

Add ref to components (#4373)

3d7166b

* Add ref to FormButton * Add ref to FormCheckbox * Add forwardRef to FormGroup * Remove accidental `only` calls

chore(FormField): use React.forwardRef() (#4359)

cd2b8f4

* Add ref to form field component * Move ref to controlProps * Add more forwardsRef tests * Remove unnecessary describe wrapper * update tests Co-authored-by: Oleksandr Fediashov <[email protected]>

chore(FormTextArea): use React.forwardRef() (#4405)

9738a3e

chore(FormRadio): use React.forwardRef() (#4406)

1b97e49

chore(FormInput): use React.forwardRef() (#4407)

d8c1c98

layershifter force-pushed the next-v3 branch from 88974b5 to 6c03c11 Compare December 13, 2022 12:44

vercel bot deployed to Preview December 13, 2022 12:50 View deployment

levithomason merged commit 95ec819 into master Feb 28, 2023

levithomason deleted the next-v3 branch February 28, 2023 17:04

dominikdosoudil reviewed Nov 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use React.forwardRef in all components #4233

feat: use React.forwardRef in all components #4233

layershifter commented Jul 23, 2021 •

edited

Loading

github-actions bot commented Jul 23, 2021 •

edited

Loading

codecov-commenter commented Jul 23, 2021 •

edited

Loading

sunech commented Dec 13, 2022

felixmosh commented Dec 13, 2022

PerryRylance commented Dec 13, 2022

layershifter commented Dec 14, 2022

levithomason commented Feb 28, 2023

levithomason commented Feb 28, 2023

layershifter commented Feb 28, 2023

felixmosh commented Feb 28, 2023

layershifter commented Feb 28, 2023

sunech commented Mar 4, 2023

layershifter commented Jul 9, 2023

dominikdosoudil Nov 12, 2024

layershifter Nov 12, 2024

dominikdosoudil Nov 12, 2024

feat: use React.forwardRef in all components #4233

feat: use React.forwardRef in all components #4233

Conversation

layershifter commented Jul 23, 2021 • edited Loading

github-actions bot commented Jul 23, 2021 • edited Loading

size-limit report

codecov-commenter commented Jul 23, 2021 • edited Loading

Codecov Report

sunech commented Dec 13, 2022

felixmosh commented Dec 13, 2022

PerryRylance commented Dec 13, 2022

layershifter commented Dec 14, 2022

levithomason commented Feb 28, 2023

levithomason commented Feb 28, 2023

layershifter commented Feb 28, 2023

felixmosh commented Feb 28, 2023

layershifter commented Feb 28, 2023

sunech commented Mar 4, 2023

layershifter commented Jul 9, 2023

dominikdosoudil Nov 12, 2024

Choose a reason for hiding this comment

layershifter Nov 12, 2024

Choose a reason for hiding this comment

dominikdosoudil Nov 12, 2024

Choose a reason for hiding this comment

layershifter commented Jul 23, 2021 •

edited

Loading

github-actions bot commented Jul 23, 2021 •

edited

Loading

codecov-commenter commented Jul 23, 2021 •

edited

Loading