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

fix: handle disabled button styles #1061

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Dec 11, 2023

They weren't well adjusted. We have to rationalize quickly
these themes issues.
This commit is more a quick fix.
Some buttons in the app don't use the disabled style some do.
Also we need 1 disabled style per variant per theme (that we use in the
app though).

image

@acezard acezard force-pushed the fix--handle-disabled-button-styles branch from 709e226 to c8e8301 Compare December 12, 2023 09:10
disabled={secondInput.length !== 4}
>
<Typography
color={secondInput.length !== 4 ? 'disabled' : 'primary'}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: const isPasswordComplete = (password) => password.length === 4 to factorize and make it very clear

@@ -18,7 +18,7 @@ export const Button = ({
style={[
styles.button,
styles[variant],
disabled ? styles.disabled : {},
disabled && styles[`${variant}Disabled`],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to check with @Ldoppea who also worked on this here 272093d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bit hard to update, @Ldoppea I think the best way to handle that is you just overwrite my styles during conflict. I update to follow same naming convention in styles though

They weren't well adjusted. We have to rationalize quickly
these themes issues.
This commit is more a quick fix.
Some buttons in the app don't use the disabled style some do.
Also we need 1 disabled style per variant per theme (that we use in the
app though).
@acezard acezard force-pushed the fix--handle-disabled-button-styles branch from c8e8301 to 129984d Compare December 12, 2023 13:53
@acezard acezard merged commit b11fb15 into master Dec 12, 2023
1 check passed
@acezard acezard deleted the fix--handle-disabled-button-styles branch December 12, 2023 14:03
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