-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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-percentage #8684
fix-percentage #8684
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Added percentage symbol display functionality to number fields in the settings form when the field type is set to 'percentage', improving visual clarity for percentage value representation.
- Modified
packages/twenty-front/src/modules/settings/data-model/fields/forms/number/components/SettingsDataModelFieldNumberForm.tsx
to include % symbol in preview display - Added conditional rendering logic to append % for percentage type fields
- Note: TODO comment regarding maxValue needs attention in future updates
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -69,7 +69,7 @@ export const SettingsDataModelFieldNumberForm = ({ | |||
<SettingsOptionCardContentCounter | |||
Icon={IconDecimal} | |||
title="Number of decimals" | |||
description={`Example: ${(1000).toFixed(count)}`} | |||
description={`Example: ${(1000).toFixed(count)} ${type === 'percentage' ? '%' : ''}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider formatting the example number (1000) differently for percentage display - 1000% might be confusing to users
@@ -69,7 +69,7 @@ export const SettingsDataModelFieldNumberForm = ({ | |||
<SettingsOptionCardContentCounter | |||
Icon={IconDecimal} | |||
title="Number of decimals" | |||
description={`Example: ${(1000).toFixed(count)}`} | |||
description={`Example: ${(1000).toFixed(count)} ${type === 'percentage' ? '%' : ''}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description={`Example: ${(1000).toFixed(count)} ${type === 'percentage' ? '%' : ''}`} | |
description={`Example: ${(type === 'percentage' ? 99 : 1000).toFixed(count)} ${type === 'percentage' ? '%' : ''}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @guillim for your contribution! |
Following previous release, a suggestion was noticed by @martmull . Here is a suggested fix.
Before :
After :