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

feat(ui-number-input): add renderIcons prop #1819

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

szalonna
Copy link
Contributor

@szalonna szalonna commented Dec 10, 2024

Closes: CLX-261

Add renderIcons prop to make the increase/decrease buttons customizable with externally defined icons
TEST PLAN:
Chromatic test for rendered icons and unit test to ensure unchanged behaviour

Copy link

github-actions bot commented Dec 10, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-12-11 14:05 UTC

@szalonna szalonna self-assigned this Dec 10, 2024
@matyasf matyasf requested review from HerrTopi and ToMESSKa December 10, 2024 10:36
@@ -226,7 +234,11 @@ const propTypes: PropValidators<PropKeys> = {
onKeyDown: PropTypes.func,
inputMode: PropTypes.oneOf(['numeric', 'decimal', 'tel']),
textAlign: PropTypes.oneOf(['start', 'center']),
allowStringValue: PropTypes.bool
allowStringValue: PropTypes.bool,
renderIcons: PropTypes.shape({
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the "renderIcons" prop to the allowedProps list as well (see below in the code)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -166,6 +166,14 @@ type NumberInputOwnProps = {
* sets the input type to string and allows string as value
*/
allowStringValue?: boolean

/**
* Definition of the icons to be rendered for increase and decrease buttons
Copy link
Contributor

Choose a reason for hiding this comment

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

Kinda nitpicky: Since this text acts as the documentation for the prop on the insUI docs page, I would rephrase it to something like this:

"Sets the icons to be rendered for increase and decrease buttons"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@szalonna szalonna force-pushed the feature/CLX-261-numberinput-custom-icons branch from 6fa4037 to df2fc56 Compare December 10, 2024 13:33
@matyasf matyasf merged commit 7be2226 into master Dec 11, 2024
11 checks passed
@matyasf matyasf deleted the feature/CLX-261-numberinput-custom-icons branch December 11, 2024 14:05
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.

4 participants