Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update dolphin settings #212

Merged

Conversation

cnkeats
Copy link
Contributor

@cnkeats cnkeats commented Aug 8, 2021

Pushes settings from Launcher to Dolphin when Dolphin is launched. Launcher settings are preferred and used to overwrite Dolphin settings when reasonable (time to launch dolphin is not noticeably affected).

  • Replay root directory
  • Use monthly subfolders

Users that don't use the Launcher will be able to change their settings as normal. However, if everyone is eventually required to use the Launcher, the ability to change settings via Dolphin should either be removed or synced with the Launcher.

image

@NikhilNarayana NikhilNarayana changed the title Feat/update dolphin settings feat: update dolphin settings Aug 8, 2021
@cnkeats cnkeats force-pushed the feat/update-dolphin-settings branch 2 times, most recently from 4f18453 to 20acadd Compare August 27, 2021 19:17
@NikhilNarayana NikhilNarayana force-pushed the feat/update-dolphin-settings branch 2 times, most recently from cc6f073 to 1f49340 Compare September 19, 2021 10:39
src/dolphin/manager.ts Outdated Show resolved Hide resolved
src/dolphin/manager.ts Outdated Show resolved Hide resolved
src/renderer/components/PathInput.tsx Outdated Show resolved Hide resolved
src/settings/settingsManager.ts Outdated Show resolved Hide resolved
src/settings/settingsManager.ts Outdated Show resolved Hide resolved
Select
</Button>
<Tooltip title={tooltipText ?? ""}>
<span>
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to wrap the Button in a span?

Copy link
Member

Choose a reason for hiding this comment

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

I think it could be a div or a span, but the reason is because if the Button is disabled the tooltip won't show

Copy link
Contributor Author

@cnkeats cnkeats Sep 20, 2021

Choose a reason for hiding this comment

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

This is correct. The tooltip will also be disabled if its immediate child is disabled. Adding an empty wrapper element is the suggested solution.

https://mui.com/components/tooltips/

Copy link
Member

@vinceau vinceau left a comment

Choose a reason for hiding this comment

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

LGTM

@NikhilNarayana NikhilNarayana merged commit bc18c41 into project-slippi:main Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants