-
Notifications
You must be signed in to change notification settings - Fork 3
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
Not jumping to score page automatically when answering correct #429
Conversation
Not jumping to score page automatically when answering correct and there is only one page of tasks.
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.
The computation of multiplePages
feels correct to me as is. It should only be true if there's more than 1 page with words.
I think I know what you want to change (https://github.com/NDLANO/h5p-vocabulary-drill/blob/master/src/components/VocabularyDrill/VocabularyDrill.tsx#L267-L270), but
- the variable
multiplePages
is also used in two other places:- https://github.com/NDLANO/h5p-vocabulary-drill/blob/master/src/components/VocabularyDrill/VocabularyDrill.tsx#L208 and
- https://github.com/NDLANO/h5p-vocabulary-drill/blob/master/src/components/VocabularyDrill/VocabularyDrill.tsx#L660 and
you would need to check that your change doesn't cause any unwanted changed there
- you should rename the variable if its doesn't reflect what it does anymore
- you should also update the comment connected to the guard (https://github.com/NDLANO/h5p-vocabulary-drill/blob/master/src/components/VocabularyDrill/VocabularyDrill.tsx#L267)
- you could instead change https://github.com/NDLANO/h5p-vocabulary-drill/blob/master/src/components/VocabularyDrill/VocabularyDrill.tsx#L268 as that's where something seems to be disturbing you. With your change, the variable named
multiplePages
will always betrue
(unless you don't have any words set at all), and then in line 268!multiplePages
will always yield false andhandleShowResults
will never be called here, and then that whole block could go.
And if that can go, the weirdscore
computation that is based on whether one is retrying can go (don't know what this should do), and then the whole isRetrying state seems to be obsolete as well - and themaxScore
state then could say bye-bye, too, I guess.
And while I look at that: Why the is the score and maxScore stored in the user state? Both should be computable from the previous state ...
Pandora's box ...
You are of course right with the condition always returning true. The reason I didn't remove this block was to keep the navigation bar in the bottom to be able to go to result page (yes, the condition would still not make sense). And it does make sense to not have the result page when there is only one page anyway. So I will remove it to get the intended solution. Seems like the |
🎉 |
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.
Did some maintenance as well.
Not jumping to score page automatically when answering correct and there is only one page of tasks.