fix(PrimaryModal): backup password validation [WPB-11590] #18172
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #18172 +/- ##
==========================================
- Coverage 46.40% 46.38% -0.03%
==========================================
Files 796 796
Lines 25780 25790 +10
Branches 5881 5884 +3
==========================================
- Hits 11964 11963 -1
- Misses 12319 12331 +12
+ Partials 1497 1496 -1 |
| markInvalid={isBackupPasswordInvalid} | ||
| onChange={(event: React.ChangeEvent<HTMLInputElement>) => { | ||
| updatePasswordWithRules(event.target.value); | ||
| setIsBackupPasswordInvalid(false); |
There was a problem hiding this comment.
Is firing this hook with every single digit change necessary?
I'd love to see a condition that it only fires once (while true)
| @@ -55,6 +55,7 @@ export const PrimaryModalComponent: FC = () => { | |||
| const isModalVisible = currentId !== null; | |||
| const passwordValueRef = useRef<HTMLInputElement>(null); | |||
| const [isFormSubmitted, setIsFormSubmitted] = useState(false); | |||
| const [isBackupPasswordInvalid, setIsBackupPasswordInvalid] = useState(false); | |||
There was a problem hiding this comment.
We need additional state for this? 🤔
Can't we do smth like that?
const isBackupPasswordValid = useMemo(() => isValidPassword(passwordInput), [passwordInput]);
And then in error use this one?
There was a problem hiding this comment.
With this approach, the initial state is broken - when input is empty, the isBackupPasswordValid will be set to true, causing an error message to show up.
We can't make an exception for an empty state (e.g passwordInput === "" ? true : isValidPassword(passwordInput) cause then we would submit the form with no password)
I'm not happy with that logic either, I gonna refactor it in this sprint, task - https://wearezeta.atlassian.net/browse/WPB-10932
There was a problem hiding this comment.
So this can be like that: const isBackupPasswordValid = useMemo(() => passwordInput && isValidPassword(passwordInput), [passwordInput]);
And there will be easy check :)
There was a problem hiding this comment.
Maybe I'm missing something, but I think we can't do that either. Then the initial state (no value) would cause isBackupPasswordValid to be true, and we don't want that:
There was a problem hiding this comment.
But if You check also isFormSubmitted, we will don't have error on initial state. Only after submit button :)
There was a problem hiding this comment.
Now I see, missed that isFormSubmitted earlier, Friday vibes 😅 Give me a see, I'm gonna refactor this code real quick
|
echoes-hq
bot
added
the
echoes: bugs
Screenshots/Screencast (for UI changes)
before:
Screen.Recording.2024-10-18.at.08.04.41.mov
after:
Screen.Recording.2024-10-18.at.08.00.01.mov
Checklist