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/settings grouped #399

Merged
merged 13 commits into from
May 18, 2022
Merged

Feat/settings grouped #399

merged 13 commits into from
May 18, 2022

Conversation

mattyg
Copy link
Collaborator

@mattyg mattyg commented May 17, 2022

Resolves #131
Resolves #333

I'm wondering if this is a good time to remove the App Name and App Description settings? Or if we want to keep them, should we display them somewhere?

Overview

  • add group field to SettingModel
  • refactor how default period settings are copied into new periods: replace use of periodOverridable, with group SettingGroup.PERIOD_DEFAULT
  • Add sub-nav within period page to navigate to different setting groups
  • Increase height of Textarea settings to 4 rows (Make settings textareas slightly larger #333)

Just a note that I went with the following display names for the settings group NavItems:

  • Application
  • Period Defaults
  • Discord Bot

Let me know if you think the names in the mockup are preferable.

I also did not include setting ordering in this PR. Let me know if you think that should be included.

@mattyg mattyg requested a review from kristoferlund May 17, 2022 21:25
Copy link
Member

@kristoferlund kristoferlund left a comment

Choose a reason for hiding this comment

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

N I C E !

I'm wondering if this is a good time to remove the App Name and App Description settings? Or if we want to keep them, should we display them somewhere?

Yeah, I don't see us using them anywhere, please toast them.

I also did not include setting ordering in this PR. Let me know if you think that should be included.

That can definitely wait.

kristoferlund
kristoferlund previously approved these changes May 18, 2022
Copy link
Member

@kristoferlund kristoferlund left a comment

Choose a reason for hiding this comment

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

👍👍

@kristoferlund kristoferlund merged commit 20f6a5b into main May 18, 2022
@kristoferlund kristoferlund deleted the feat/settings_grouped branch May 18, 2022 19:51
@kristoferlund kristoferlund mentioned this pull request May 18, 2022
@kristoferlund kristoferlund mentioned this pull request May 31, 2022
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.

Make settings textareas slightly larger Improve Settings page UX
2 participants