Deck selection dialog remembers location when reopening #9935

[souravmunjal]

Pull Request template

Purpose / Description

Previously after selecting the deck from the deck selection dialog the scroll position was lost when the dialog was closed, If we again open the dialog it should be scrolled down to the deck selected

Fixes

Fixes #9935

Approach

I configured the recycler view, that was showing the deck list ,to scroll down to the particular selected deck. Basically the DeckSelectionDialog class is called from function of these 2 classes. 1 DeckSelectionSpinner 2 CardTemplateEditor So I made some necessary changes in above 3 files to ensure the above feature and also i also updated DeckNameComparotor.kt acc. to latest kotlin functions which i found was depreciated. In total 4 files were changed

How Has This Been Tested?

You can test it through going statitcs, or Add Notes->Cards->Deck Override The deck selection dialog was called from many other places but these are the screens where it was called from DeckSelectionSpinner and CardTemplateEditor respectively, so both the cases are test and working fine

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration (SDK version(s), emulator or physical, etc)

Learning (optional, can help others)

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

• [done] You have not changed whitespace unnecessarily (it makes diffs hard to read)
• [ done] You have a descriptive commit message with a short title (first line, max 50 chars).
• [ done] Your code follows the style of the project (e.g. never omit braces in if statements)
• [ done] You have commented your code, particularly in hard-to-understand areas
• [ done] You have performed a self-review of your own code
• [done ] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• [ ] UI Changes: You have tested your change using the Google Accessibility Scanner

[souravmunjal]

Thanks for posting this! It has some issues with formatting - you can see it doesn't follow the conventions of code nearby, our style is documented here https://github.com/ankidroid/Anki-Android/wiki/Code-style (as we move to kotlin this will be automatically enforced but for the legacy java files we still have to do it manually) The static thing is troublesome, though I understand based on your note why - because newInstance on the spinner is static. That needs some thought, in case it's possible for the calling objects to simply know at the time what the selected deck should be . It may be that the "currently selected deck" is sufficient, but I haven't thought through this thoroughly yet

@mikehardy any updates on this? left some comments on the pr

[mikehardy]

I saw your comments. I left my own, you have not addressed them So I could ask,

@souravmunjal any updates on this? left some comments on the pr

:thinking:

[souravmunjal]

I saw your comments. I left my own, you have not addressed them So I could ask,

@souravmunjal any updates on this? left some comments on the pr

🤔

oh i thought you were going to review on that 😅 my bad so i read the doc i feel it is according to that only but you find any issues can you pin it like the namings of the variable( which are selectedDeck,currentSelecttedDeckId) or anything else it would be great help

[souravmunjal]

Thanks!

Went through the code style for you, could you keep the changes in mind for your next PR?

yeah yeah sure will update that this line will get affected in line no 747 CardTemplateEditor.java if we not keep that variable static DeckSelectionDialog dialog = DeckSelectionDialog.newInstance(title, explanation,(selectedDeck!=null) ?selectedDeck.getDeckId():ALL_DECKS_ID,true, decks); AnkiActivity.showDialogFragment(activity, dialog);

[david-allison]

this line will get affected in line no 747 CardTemplateEditor.java if we not keep that variable static DeckSelectionDialog dialog = DeckSelectionDialog.newInstance(title, explanation,(selectedDeck!=null) ?selectedDeck.getDeckId():ALL_DECKS_ID,true, decks); AnkiActivity.showDialogFragment(activity, dialog);

Thanks! It's best to reply to individual comments in the threads: https://github.com/ankidroid/Anki-Android/pull/9970#discussion_r766269692

[souravmunjal]

updated the code please check

[mikehardy]

I thought the title was meaningless and the description did not have enough information to give the reviewer context, I changed them I'll queue this for re-review

[balta2ar]

@mikehardy friendly reminder :)

[mikehardy]

Has a conflict now so needs a rebase to master, it appears to me whenever I check this that there were comments after the last comments from David and that the comments weren't resolved, so this isn't ready for review from anyone else yet

[github-actions[bot]]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[balta2ar]

Ping @souravmunjal

[github-actions[bot]]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[balta2ar]

Ping @souravmunjal

[github-actions[bot]]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[balta2ar]

sigh ping @souravmunjal

[david-allison]

sigh #9935 is closed. Why is this being bumped?