Skip to content

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented Oct 14, 2024

This renders most drop-down selection prompts inside Heroic. In turn, these dropdowns now work inside the Gamescope session.

Current issues:

  • Controller navigation does not work, hitting "Down" opens the dropdown, with no way to navigate further down to other components
  • There is a black box in the top-left of the component, where the label should be
  • The dropdowns do not work while in a Dialog (they're rendered behind the Dialog)

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

This renders most drop-down selection prompts inside Heroic. In turn, these
dropdowns now work inside the Gamescope session.

Current issues:
- Controller navigation does not work, hitting "Down" opens the dropdown, with
  no way to navigate further down to other components
- There is a black box in the top-left of the component, where the label should
  be
@CommandMC CommandMC added the pr:wip WIP, don't merge. label Oct 14, 2024
@CommandMC CommandMC self-assigned this Oct 14, 2024
MUI CSS class names are not stable, they cannot be used like this
I prefer to use this over CSS rules in a separate file since it can sometimes be
unclear where "the magic happens". Having layout and style together is clearer
IMO
This will definitely break things, but it makes selects work
Navigating "over" selects now works

This is done by simulating Tab/Shift-Tab key presses when pressing the
respective buttons while focussing a MUI select element

I've gone ahead and removed our home-grown `GamepadInputEvent` type, as it's
just a lesser version of Electron's input events
This makes dialogs closeable with the back button again
Restoring the focussed element on dialog close is already handled by MUI, we
don't need this code ourselves

Moving focus on dialog open *should* work automatically as well, however it
doesn't. I've tried various methods with element properties and JS, nothing
seems to work. As a band-aid fix for now, we simulate 1-2 tab presses to focus
the dialog content
Closing selects with the B/Circle button
MUI treats an empty value as "don't show anything selected", while native
selects show the first option
@CommandMC CommandMC added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:wip WIP, don't merge. labels Dec 5, 2024
@CommandMC CommandMC marked this pull request as ready for review December 5, 2024 21:11
@CommandMC CommandMC requested a review from a team December 5, 2024 21:12
@CommandMC CommandMC requested review from arielj, flavioislima, Nocccer and imLinguin and removed request for a team December 5, 2024 21:12
Copy link
Collaborator

@arielj arielj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I left a comment about an accessibility feature that is not working with the new dialogs

Copy link
Collaborator

@arielj arielj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one more thing I noticed, when using a RTL language, the arrow in the select field is in the wrong side of the element:

in main:
image

with these changes:
image

@arielj
Copy link
Collaborator

arielj commented Jan 6, 2025

I've been testing a bit more, I found 1 issue and two small things:


one small thing is the categories and filters style changed and it's missing some padding:

  • main:
    image

  • this branch:
    image


the second small thing is that the controller hints (the instructions of what each button does at the bottom of the screen) shows incorrect hints for dropdowns, it thinks it's the context menu of library cards and it shows ( x ) Close Options incorrectly. This one can be fixed by using target.closest('.contextMenu') in line 60 of src/frontend/components/UI/ControllerHints/index.tsx


now the issue is that, when using a controller to navigate the UI (or a keyboard), you can get stuck focusing the X button of dialogs when the dialog's content is long and it shows a scrollbar:

  • open the settings for a game
  • in the wine tab (because it's long it will show a scrollbar), move up until you focus the X
  • you can't move out of there (moving up/down scrolls the library page, moving left/right does nothing)

a smaller dialog doesn't have this problem:

  • now use the mouse and change to the other tab (this is short and won't show a scrollbar)
  • move up until you focus the X
  • you can move out (moving up does nothing as expected, down/left/right moves focus to the tabs)

@CommandMC CommandMC force-pushed the feat/web-dropdowns branch from 1f72fc7 to 19704e7 Compare May 1, 2025 15:21
CommandMC added 4 commits May 1, 2025 17:32
Ideally we'd move `.selectStyle` into its own CSS file (since it's only used for
those two buttons now), but I'm not sure what the best way to do that would be
This makes the controller hints show up over dialogs
@CommandMC CommandMC requested a review from arielj May 1, 2025 16:14
@flavioislima flavioislima requested a review from Copilot May 14, 2025 10:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates drop-down selection components to render as Material UI Selects, enhancing controller navigation and dialog behavior within the Gamescope session. Key changes include replacing native options with MUI MenuItem components, refining gamepad navigation actions, and refactoring dialog components to use MUI's Dialog APIs. Reviewed Changes Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments. Show a summary per file File Description src/frontend/screens/Settings/components/CustomWineProton.tsx Replaced native with MUI MenuItem for custom wine paths. src/frontend/screens/Library/components/InstallModal/* and related selector files Updated drop-down implementations in InstallModal sub-components. src/frontend/components/UI/SelectField/* and ThemeSelector/index.tsx Migrated from native select elements to MUI Select components. src/frontend/helpers/gamepad.ts Added new gamepad actions (tab/shiftTab) and helper functions. src/frontend/components/UI/Dialog/* Refactored dialog components to leverage MUI's Dialog for better UX. src/frontend/components/UI/ControllerHints/index.tsx Adjusted controller hints condition for detecting menus. src/common/types.ts and src/backend/main.ts Removed legacy types and updated gamepad event handling. src/frontend/App.css Adjusted z-index values to accommodate MUI Dialog layering. Comments suppressed due to low confidence (3) src/frontend/components/UI/ControllerHints/index.tsx:60 The condition for detecting context menus changed from '.MuiMenu-list' to '.contextMenu'. Verify this update is intentional and consistent with the rest of the UI component structure. } else if (target.closest('.contextMenu')) { src/frontend/components/UI/SelectField/index.tsx:39 [nitpick] Ensure that the new MUI Select component includes all necessary accessibility attributes (e.g., aria-label) to replicate the native select behavior. <Select src/frontend/helpers/gamepad.ts:146 [nitpick] Review the handling of 'isInMuiPopover' to ensure that simulating a 'tab' action in popover contexts does not introduce unintended navigation issues. } else if (isInMuiPopover()) {

@CommandMC CommandMC force-pushed the feat/web-dropdowns branch from 80f17b8 to 0883901 Compare May 19, 2025 20:04
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CommandMC CommandMC merged commit 559ad5a into main May 19, 2025
10 of 11 checks passed
@CommandMC CommandMC deleted the feat/web-dropdowns branch May 19, 2025 22:04
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators May 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants