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(textProps): adding ability to spread TextProps to help with accessibility #62

Merged

Conversation

theonetheycallneo
Copy link
Contributor

@theonetheycallneo theonetheycallneo commented Oct 8, 2024

Adding ability to override TextProps when customizing the calendar especially for accessibility.

Resolves #61 and #36

Copy link

changeset-bot bot commented Oct 8, 2024

🦋 Changeset detected

Latest commit: a14778c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@marceloterreiro/flash-calendar Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Mookiies
Copy link

Mookiies commented Oct 9, 2024

This is especially necessary for font scaling. Without a way to do limit this the calendar text will scale leading to disastrous results

image

@danibonilha
Copy link
Contributor

This is especially necessary for font scaling. Without a way to do limit this the calendar text will scale leading to disastrous results

@Mookiies if you're only issue is font scaling and you'd like to disable for your whole app, you could add this to your index.js file:

Text.defaultProps = Text.defaultProps || {}
Text.defaultProps.allowFontScaling = false

That being said I agree this PR is important even to easily add testIDs if needed and of course allow to handle font scaling for the calendar only

@theonetheycallneo
Copy link
Contributor Author

theonetheycallneo commented Oct 15, 2024

This is especially necessary for font scaling. Without a way to do limit this the calendar text will scale leading to disastrous results

image

This is especially necessary for font scaling. Without a way to do limit this the calendar text will scale leading to disastrous results

@Mookiies if you're only issue is font scaling and you'd like to disable for your whole app, you could add this to your index.js file:

Text.defaultProps = Text.defaultProps || {}
Text.defaultProps.allowFontScaling = false

That being said I agree this PR is important even to easily add testIDs if needed and of course allow to handle font scaling for the calendar only

@danibonilha I appreciate the input and we are aware of disabling font scaling across the entire app. Unfortunately, that's not an option as out app requires additional accessibility and specific textfields need to scale while others are restricted.

Copy link
Owner

@MarceloPrado MarceloPrado left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the contribution 🙏🏻
Just one feedback before we land this

packages/flash-calendar/src/components/CalendarItemDay.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@MarceloPrado MarceloPrado left a comment

Choose a reason for hiding this comment

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

one last feedback @theonetheycallneo

Copy link
Owner

@MarceloPrado MarceloPrado left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @theonetheycallneo! Looks great

@MarceloPrado MarceloPrado merged commit f3b9ec8 into MarceloPrado:main Oct 27, 2024
1 check passed
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.

feat(text): Ability to pass TextProps to better control the <Text> days and weekdays
4 participants