Skip to content

Moved SliderGroup to FC. #2832

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

Merged
merged 5 commits into from
Dec 6, 2023
Merged

Moved SliderGroup to FC. #2832

merged 5 commits into from
Dec 6, 2023

Conversation

nitzanyiz
Copy link
Collaborator

@nitzanyiz nitzanyiz commented Nov 29, 2023

Description

Converted SliderGroup to FC.

Changelog

SliderGroup - Moved to a FC.

Additional info

@nitzanyiz nitzanyiz changed the title To fc/slider group SliderGroup conversion to FC Nov 29, 2023
@nitzanyiz nitzanyiz changed the title SliderGroup conversion to FC Moved SliderGroup to FC. Nov 30, 2023
@nitzanyiz nitzanyiz marked this pull request as ready for review November 30, 2023 09:14
@nitzanyiz nitzanyiz requested a review from Inbal-Tish November 30, 2023 14:59
@Inbal-Tish Inbal-Tish self-assigned this Dec 5, 2023
@Inbal-Tish
Copy link
Collaborator

@nitzanwix Please add the "changelog" filed on all PRs


constructor(props: SliderGroupProps) {
super(props);
const valueSetter = useCallback((value: tinycolor.ColorFormats.HSLA) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

'valueSetter' is not a proper name for a callback. We usually use the name of the prop it is passed to or as an action describing what this method does

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is _setValue better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes

@nitzanyiz nitzanyiz requested a review from Inbal-Tish December 5, 2023 11:49
@Inbal-Tish Inbal-Tish merged commit 3c1a0f3 into master Dec 6, 2023
@nitzanyiz nitzanyiz deleted the toFC/SliderGroup branch December 6, 2023 11:00
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