[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP #27449

stephanieelliott · 2023-09-14T14:50:35Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

Click CMD + J to open the keyboard shortcuts menu

Expected Result:

The keyboard shortcuts menu opens in the RHP

Actual Result:

The keyboard shortcuts menu opens as a modal in the middle of the browser window like this:

Workaround:

n/a

Platforms:

Web
Desktop App

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~018fa7bda44a6beb13
Upwork Job ID: 1704930075112583168
Last Price Increase: 2023-09-21
Automatic offers:

samh-nl | Contributor | 26988957

melvin-bot · 2023-09-14T14:50:40Z

Triggered auto assignment to @kadiealexander (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

melvin-bot · 2023-09-14T14:50:41Z

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

stephanieelliott · 2023-09-14T14:52:16Z

Tagging in @shawnborton for mockup of what the shortcut menu should look like in the RHP

samh-nl · 2023-09-14T15:13:15Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Display keyboard shortcuts menu in the RHP

What is the root cause of that problem?

The keyboard shortcuts modal uses a deviating implementation compared to other pages.

What changes do you think we should make in order to solve the problem?

Remove <KeyboardShortcutsModal /> from Expensify.js
In main.js, rename showKeyboardShortcutsModal to showKeyboardShortcutsPage
In main.js, rename ELECTRON_EVENTS.SHOW_KEYBOARD_SHORTCUTS_MODAL to SHOW_KEYBOARD_SHORTCUTS_PAGE
Make a new page component KeyboardShortcutsPage and add it to both ModalStackNavigators and RightModalNavigator
In AuthScreens, listen to the keyboard shortcut and navigate to the page when triggered, in a similar fashion to how we do so for the search page. We should define a new route, e.g. ROUTES.KEYBOARD_SHORTCUTS.

App/src/libs/Navigation/AppNavigator/AuthScreens.js

Lines 167 to 180 in 95c9e47

    
           this.unsubscribeSearchShortcut = KeyboardShortcut.subscribe( 
        
               searchShortcutConfig.shortcutKey, 
        
               () => { 
        
                   Modal.close(() => { 
        
                       if (Navigation.isActiveRoute(ROUTES.SEARCH)) { 
        
                           return; 
        
                       } 
        
                       return Navigation.navigate(ROUTES.SEARCH); 
        
                   }); 
        
               }, 
        
               searchShortcutConfig.descriptionKey, 
        
               searchShortcutConfig.modifiers, 
        
               true, 
        
           );

Similar to the current modal, we fetch the shortcuts from KeyboardShortcut.getDocumentedShortcuts() upon mount and set this to state. Subsequently, we iterate and render the shortcuts. E.g.:

const renderShortcut = (shortcut) => (
    <View key={shortcut.displayName} style={styles.mb5}>
        <View style={styles.mb1}>
            <Text>{shortcut.displayName}</Text>
        </View>
        <View>
            <Text style={styles.textLabelSupporting}>{translate(`keyboardShortcutModal.shortcuts.${shortcut.descriptionKey}`)}</Text>
        </View>
    </View>
);

Finally, we cleanup various things that are no longer relevant, including:

StyleUtils.getKeyboardShortcutsModalWidth
Style definitions: styles.keyboardShortcutModalContainer, styles.keyboardShortcutTableWrapper, styles.keyboardShortcutTableContainer
ONYXKEYS.IS_SHORTCUTS_MODAL_OPEN
The file KeyboardShortcuts.js in libs/actions/

What alternative solutions did you explore? (Optional)

Alternatively, we can implement it as a right-docked modal. However, this does not inherently add a backdrop (<Overlay />). This could be added, but extra logic would have to be put in place to ensure it does not cause a double backdrop in the case that the RHP was already open.

In KeyboardShortcutsModal, we change modalType to CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED.
Remove the isSmallScreenWidth prop
Remove the innerContainerStyle prop that is passed to the Modal component
To render the left-arrow rather than close button, remove shouldShowCloseButton and shouldShowBackButton={false} from HeaderWithBackButton. Furthermore, replace onCloseButtonPress with onBackButtonPress.
Remove the <View style={[styles.keyboardShortcutTableWrapper]}> wrapper.
Remove the styles (i.e. styles.keyboardShortcutTableContainer) applied to the inner View component.
Update renderRow (same as step 6 above).
In styles.js/StyleUtils, cleanup the removed styles.

shawnborton · 2023-09-15T08:03:25Z

I think something simple like this would do the trick, cc @Expensify/design

samh-nl · 2023-09-15T09:50:40Z

We currently toggle the modal if CMD+J is pressed again. Is it the intention to maintain this behavior (strictly speaking, the description only states that it will open it)?

I have also updated my proposal with some additional details.

puneetlath · 2023-09-15T14:37:00Z

I think it'd be like CMD+K. It opens the page, but doesn't close it.

melvin-bot · 2023-09-18T09:27:54Z

@shawnborton, @stephanieelliott Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2023-09-20T09:54:41Z

@shawnborton, @stephanieelliott Eep! 4 days overdue now. Issues have feelings too...

shawnborton · 2023-09-21T03:08:19Z

I think we can take this off hold and start moving things forward.

stephanieelliott · 2023-09-21T18:45:51Z

Thanks for the mock @shawnborton! Taking this off hold and adding the External label

melvin-bot · 2023-09-21T18:46:23Z

Job added to Upwork: https://www.upwork.com/jobs/~018fa7bda44a6beb13

melvin-bot · 2023-09-21T18:46:25Z

Current assignee @stephanieelliott is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-10-11T22:20:24Z

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

when @samh-nl got assigned: 2023-10-02 22:17:46 Z
when the PR got merged: 2023-10-11 22:20:24 UTC
days elapsed: 7

On to the next one 🚀

melvin-bot · 2023-10-13T22:06:30Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-10-13T22:06:34Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.83-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-10-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@eVoloshchak does not require payment (Eligible for Manual Requests)
@samh-nl requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-10-13T22:58:00Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.83-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-10-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@eVoloshchak does not require payment (Eligible for Manual Requests)
@samh-nl requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-10-16T21:20:31Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@eVoloshchak does not require payment (Eligible for Manual Requests)
@samh-nl requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-10-16T21:46:17Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@eVoloshchak does not require payment (Eligible for Manual Requests)
@samh-nl requires payment offer (Contributor)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

eVoloshchak · 2023-10-23T10:59:58Z

Requested the payment via NewDot

stephanieelliott · 2023-10-23T17:30:45Z

Summarizing payment on this issue:

Issue reporter: N/A
Contributor: @samh-nl $500 (pay via Upwork)
Contributor+: @eVoloshchak $500 (request via NewDot)

Upwork job is here, no bonus is included on final payout

JmillsExpensify · 2023-10-23T17:43:15Z

$500 payment approved for @eVoloshchak based on summary above.

stephanieelliott · 2023-10-25T21:19:34Z

All paid!

melvin-bot · 2024-02-02T19:26:59Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

stephanieelliott added Daily KSv2 NewFeature Something to build that is a new item. Design labels Sep 14, 2023

melvin-bot bot assigned kadiealexander Sep 14, 2023

melvin-bot bot assigned shawnborton Sep 14, 2023

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Sep 14, 2023

stephanieelliott assigned stephanieelliott and unassigned kadiealexander Sep 14, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 14, 2023

stephanieelliott changed the title ~~[$500] Display keyboard shortcuts menu in the RHP~~ [HOLD] Display keyboard shortcuts menu in the RHP Sep 14, 2023

melvin-bot bot added the Overdue label Sep 18, 2023

melvin-bot bot removed the Overdue label Sep 21, 2023

stephanieelliott added the External Added to denote the issue can be worked on by a contributor label Sep 21, 2023

melvin-bot bot changed the title ~~[HOLD] Display keyboard shortcuts menu in the RHP~~ [$500] [HOLD] Display keyboard shortcuts menu in the RHP Sep 21, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 21, 2023

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 13, 2023

melvin-bot bot changed the title ~~[$500] Display keyboard shortcuts menu in the RHP~~ [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP Oct 13, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Oct 13, 2023

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 13, 2023

melvin-bot bot changed the title ~~[HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP~~ [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP Oct 13, 2023

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 16, 2023

melvin-bot bot changed the title ~~[HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP~~ [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP Oct 16, 2023

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 16, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Oct 20, 2023

melvin-bot bot removed the Overdue label Oct 23, 2023

stephanieelliott closed this as completed Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP #27449

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP #27449

stephanieelliott commented Sep 14, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

stephanieelliott commented Sep 14, 2023

samh-nl commented Sep 14, 2023 •

edited

Loading

shawnborton commented Sep 15, 2023

samh-nl commented Sep 15, 2023 •

edited

Loading

puneetlath commented Sep 15, 2023

melvin-bot bot commented Sep 18, 2023

melvin-bot bot commented Sep 20, 2023

shawnborton commented Sep 21, 2023

stephanieelliott commented Sep 21, 2023

melvin-bot bot commented Sep 21, 2023

melvin-bot bot commented Sep 21, 2023

melvin-bot bot commented Oct 11, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

eVoloshchak commented Oct 23, 2023

stephanieelliott commented Oct 23, 2023

JmillsExpensify commented Oct 23, 2023

stephanieelliott commented Oct 25, 2023

melvin-bot bot commented Feb 2, 2024

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP #27449

[HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-20] [HOLD for payment 2023-10-20] [$500] Display keyboard shortcuts menu in the RHP #27449

Comments

stephanieelliott commented Sep 14, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Sep 14, 2023

melvin-bot bot commented Sep 14, 2023

stephanieelliott commented Sep 14, 2023

samh-nl commented Sep 14, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

shawnborton commented Sep 15, 2023

samh-nl commented Sep 15, 2023 • edited Loading

puneetlath commented Sep 15, 2023

melvin-bot bot commented Sep 18, 2023

melvin-bot bot commented Sep 20, 2023

shawnborton commented Sep 21, 2023

stephanieelliott commented Sep 21, 2023

melvin-bot bot commented Sep 21, 2023

melvin-bot bot commented Sep 21, 2023

melvin-bot bot commented Oct 11, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 13, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

eVoloshchak commented Oct 23, 2023

stephanieelliott commented Oct 23, 2023

JmillsExpensify commented Oct 23, 2023

stephanieelliott commented Oct 25, 2023

melvin-bot bot commented Feb 2, 2024

stephanieelliott commented Sep 14, 2023 •

edited by melvin-bot bot

Loading

samh-nl commented Sep 14, 2023 •

edited

Loading

samh-nl commented Sep 15, 2023 •

edited

Loading