frontend: fix continue button not clickable in wizard #1160
Conversation
| const target = event.target as HTMLInputElement; | ||
| const name = target.id as 'argement1' | 'argement2' | 'argement3' | 'argement4' | 'argement5'; | ||
| this.setState(prevState => ({ | ||
| ...prevState, |
There was a problem hiding this comment.
i will never understand what this does and what a simple this.setState({ [name]: target.checked }) would do wrong here. Can you enlighten me? :D
There was a problem hiding this comment.
The normal reason why you would do that is if you want to change the state based on the current state, i.e. toggle something on/off, this would be important if you click faster than react updating the state. In that case we can pass an async function instead of an object.
see
https://reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous
// bad
this.setState({
counter: this.state.counter + this.props.increment,
});
// good
this.setState((state, props) => ({
counter: state.counter + props.increment
}));but in this particular case, it is because TypeScript didn't like it... I didn't really deeply investigate deeply why 🙈
There was a problem hiding this comment.
There was a problem hiding this comment.
It seems Typescript doesn't like ES6 style dynamic property keys, but doesn't complain about oldschool, tried this and this
With old school style TypeScript doesn't complain, but also doesn't help.
const obj = {};
obj[target.id] = target.checked;
this.setState(obj);This also works, but I have no idea what it means, and if it would help in any useful way
this.setState({
[target.id]: target.checked
} as unknown as Pick<State, keyof State>);from the same guy, because it is "shorter"
this.setState<never>({
[target.id]: target.checked
});Back to the oldschool, I added as 'argeement1 | 'argeement2'' doesn't help, but at least the intention is human readable.
const obj = {};
obj[target.id as 'agreement1' | 'agreement2' | 'agreement3' | 'agreement4' | 'agreement5'] = target.checked;
this.setState(obj);| @@ -605,7 +610,7 @@ class BitBox02 extends Component<Props, State> { | |||
| <div className={style.stepContext}> | |||
| <p>{t('bitbox02Wizard.stepBackup.createBackup')}</p> | |||
| <p className="m-bottom-default">{t('bitbox02Wizard.stepBackup.beforeProceed')}</p> | |||
| <form ref={this.setDisclaimerRef}> | |||
| <form> | |||
| <div className="m-top-quarter"> | |||
| <Checkbox onChange={this.handleDisclaimerCheck} className={style.wizardCheckbox} id="agreement1" label={t('bitbox02Wizard.backup.userConfirmation1')} /> | |||
There was a problem hiding this comment.
don't you also need to put checked={agreement1} here and same with all others? e.g. like described here:
https://reactjs.org/docs/forms.html#handling-multiple-inputs
There was a problem hiding this comment.
Thanks, yes you are right
There was a problem hiding this comment.
Unfortunatelly I could only reproduce it once and got the error js: Uncaught TypeError: Cannot read property 'querySelectorAll' of null on line https://github.com/digitalbitbox/bitbox-wallet-app/blob/6ed4ff598ce0a352e13bb5bc3a42edcebc83d47f/frontends/web/src/components/devices/bitbox02/bitbox02.tsx#L357 This fix removes the dom reference and instead adds each checkbox as a boolean to the state. That way we don't depend on the form to be there or having to querySelect the input elements and check if they are checked.
thisconnect
force-pushed
the
frontend-fix-wizardcontinue
branch
from
fd680ef
to
f200788
Compare
Unfortunatelly I could only reproduce it once and got the error
js: Uncaught TypeError: Cannot read property 'querySelectorAll' of null
on line
bitbox-wallet-app/frontends/web/src/components/devices/bitbox02/bitbox02.tsx
Line 357 in 6ed4ff5
This fix removes the dom reference and instead adds each checkbox
as a boolean to the state. That way we don't depend on the form
to be there or having to querySelect the input elements and check
if they are checked.