Gui refactored into separate views | rewrite of keydown handler into event handler #873
Conversation
…s cluttered and not a monolithic file
… back wasn't shown
|
Hi @KyleKlus thanks so much for doing this, especially with the detailed explanations on #870. Hopefully I'll get a chance to review over the next couple of days. I kind of got excited about #748, and how much research has gone into the https://github.com/open-spaced-repetition/fsrs4anki algorithm. Actually, would love your thoughts regarding #851 whilst you are "in the neighborhood" Cheers |
|
The new algorithm in #748 really looks nice and I think it could be a great benefit for the users. Maybe it could be first implemented as an opt-in feature so it can be tested first, how it behaves and feels. I also had the same idea as mentioned in #851, but just for the normal review mode; I think it is particularly useful, so that one roughly knows how long the learning session might still take. And it would be very easy to implement it, as all data is already there and the ui has still some space for a subheading. As for the random mode, your way of showing the remaining count is a better metric to go by and I think we can even remove the number of cards in the subdeck, as they are not relevant in this mode; only in the other modes. I am going to think a bit about how to show this information, maybe I'll find a better way of presenting this 🤔. The only thing to consider with the subheading is a possible jump in the ui, if the root deck has cards and then after those are done it switches to the subdeck. The subheading could push the question section down if it appears, so I would pre allocate the space for the sub deck info, so it doesn't push anything down when it appears. I would be happy to implement that after I've made the other ui changes in #872 😊. |
I was thinking the same 👍
Good point, I hadn't thought of that
I think I understand your point, personally it wouldn't bother me if occasionally after completing a card during the review, the next card is displayed at a slightly different vertical position because of the extra info. I only use Obsidian on the desktop so vertical space isn't a premium, as long as on a mobile the pre allocated space doesn't seem wasted when it's blank.
Awesome! |
|
Hi again, I am done now with all the ui improvements I wanted to do in #872. The PR for the this would build upon this unmerged PR, as I finalized/improved the view switching a bit there 🚀 (Link to the changes). So would the reviewing process be easier if I wait until this one is finished or would it be better, if I close this one and open another one with all the ui changes at once? I just don't want to overwhelm the review process with too many changes 😇. |
|
Hi @KyleKlus, I like your idea to make the UI changes under a separate PR. That way this PR is purely for the refactoring. Cheers |
|
Thanks for refactoring On this page lint lists unused variables which it considers a warning: I'm surprised that the lint test above indicates that it passed. It seems that it doesn't consider warnings as a reason to block the merge. Would be good if you looked at the warnings and fixed the code (usually simple things - remove unused variables, use Also I'm wanting to check on my Android phone, but can't work out how to overwrite the Cheers |
|
I think, I fixed now all the warnings. Sry, that I didn't removed the unused variables; I don't know why I forgot that 😅. |
|
LGTM, thanks! |
Description
This PR implements a refactor of the flashcard-modal.tsx into serveral view files (DecksListView, FlashcardReviewView).
This was done to ensure code maintainability for possible later overhauls (eg. issue #872). Like this every view has its separate file, where only the code for this view is put.
Another thing, which this PR implements is the rewritten keydown handler. Now it is written as proper event listener, with extensibility (Through switch statements) and readability in mind.
Changes