-
Notifications
You must be signed in to change notification settings - Fork 164
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
#5200 – the users choice is not saved in the settings but is recalculated in px #5298
#5200 – the users choice is not saved in the settings but is recalculated in px #5298
Conversation
…choice-is-not-saved-in-the-settings-but-is-recalculated-in-px
name, | ||
className, | ||
...rest | ||
}) => { | ||
const [measure, setMeasure] = useState('px'); | ||
const [measure, setMeasure] = useState( | ||
extraValue || extraSchema?.default || 'px', |
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.
Can we use MeasurementUnits enum here instead of string literal?
}; | ||
|
||
useEffect(() => { | ||
calcValue(); | ||
}, [value, measure, calcValue]); | ||
setCust(convertValue(value, 'px', measure)); |
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.
Can we use MeasurementUnits enum here instead of 'px'?
@@ -71,18 +71,23 @@ class Form extends Component { | |||
onUpdate(instance, valid, errs); | |||
} | |||
|
|||
field(name, onChange) { | |||
field(name, onChange, extraName) { |
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.
Maybe rename 'extra' to something like 'second input name' or something that will describe it more clear?
How the feature works? / How did you fix the issue?
(Screenshots, videos, or GIFs, if applicable)
Added 5 new properties to settings
Updated settings components to have extra schema and reflect the changes in it
Check list
#1234 – issue name