[GSoC'24] CardBrowser: Shortcut E

[SanjaySargam]

Purpose / Description

This PR is a part of #16795

• [GSoC'24] CardBrowser: Implementation of Keyboard Shortcuts #16795

How Has This Been Tested?

HP Chromebook

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• 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

[david-allison]

@SanjaySargam What's pending before this becomes ready to review?

[Arthur-Milchior]

only nit. If you only change comment/documentation we can merge immediately

[Arthur-Milchior]

I admit I don't understand the "doesn't particularly matter". That does not seems obvious to me.
And in particular, it seems it should be if part of the if/else.
Because that's the only place where we consider the case where there is indeed multiple selections.

[david-allison]

What if there's no cards?

[Arthur-Milchior]

@david-allison If there is no card, it edit the first card of the search result.
Which is what I was asking him to document

[david-allison]

What if there are no cards in the deck/collection?

[SanjaySargam]

What if there are no cards in the deck/collection?

It show toast as Something went wrong

    private fun openNoteEditorForCurrentlySelectedNote() = launchCatchingTask {
        try {
            val cardId = getCardIdForNoteEditor()
            openNoteEditorForCard(cardId)
        } catch (e: Exception) {
            Timber.w(e, "Error Opening Note Editor")
            showSnackbar(
                R.string.multimedia_editor_something_wrong,
                Snackbar.LENGTH_LONG
            )
        }
    }

[david-allison]

It should do something more user-friendly. It's an expected 'error' case

[SanjaySargam]

@david-allison Should I change toast as No note to open NoteEditor?

[david-allison]

I'd go with No note to edit

[david-allison]

Unit tests would be great here

[david-allison]

Awaiting Sanjay's approval: either waiting for tests, or merging as-is

[SanjaySargam]

Unit tests would be great here

I'll add in separate PR

[david-allison]

This should only show no note to edit if there's no note to edit. Not on any failure.

[SanjaySargam]

Self high-five! Approved!

[Arthur-Milchior]

Sorry @SanjaySargam you won't get the 2nd approval right now. David make a useful remark. You should show the new message if no notes are selected. If there is any other error, then there should be no changes

[david-allison]

Beautiful, thanks for taking the time with this one

[david-allison]

Blocked on translation sync

[github-actions]

Maintainers: Please Sync Translations to produce a commit with only the automated changes from this PR.

Read more about updating strings on the wiki,

• localization-administration
• download-localized-strings