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

Settings improvements #1618

Merged
merged 11 commits into from
Jun 17, 2024
Merged

Conversation

erri120
Copy link
Member

@erri120 erri120 commented Jun 13, 2024

2024-06-13_11-44-34

@erri120 erri120 requested a review from a team June 13, 2024 09:22
@erri120 erri120 self-assigned this Jun 13, 2024
Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

There are some translations that could be made, but would likely create conflicts with the other PRs we have, we can also do these in future prs.

.WithDescription("Send anonymous analytics information and usage data to Nexus Mods.")
.AddToSection(Sections.Privacy)
.WithDisplayName("Send usage data")
.WithDescription("Help us improve the App by sending usage data to Nexus Mods.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider translation

Comment on lines +16 to +24
.AddToSection(Sections.Advanced)
.WithDisplayName("Show game files")
.WithDescription("Show game files as a mod alongside your added mods.")
.UseBooleanContainer()
)
.AddPropertyToUI(x => x.ShowOverride, propertyBuilder => propertyBuilder
.AddToSection(sectionId)
.WithDisplayName("Show Override")
.WithDescription("Shows Override in the Mods page.")
.AddToSection(Sections.Advanced)
.WithDisplayName("Show Override mod")
.WithDescription("Shows the Override mod, which contains files generated or modified during gameplay that aren't part of any specific mod.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider translations

.WithDescription("When enabled, logs will be written to the console as well as the log file.")
.AddToSection(Sections.DeveloperTools)
.WithDisplayName("Log to console")
.WithDescription("Enables the ConsoleTarget (stdout) for all loggers.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider translations

Comment on lines +26 to +27
view => view.ViewModel!.SettingEntries,
view => view.ViewModel!.Sections
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not expected to change right? This should not get triggered on every setting value change for example, right?

{
Id = Sections.General,
IconFunc = static () => IconValues.Desktop,
Name = "General",
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider translation

.WithDescription("For testing the Xbox Game Pass detection")
.AddToSection(Sections.Experimental)
.WithDisplayName("Enable Xbox Game Pass support")
.WithDescription("Allows you to manage games installed with Xbox Game Pass.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider translations

@Al12rs
Copy link
Contributor

Al12rs commented Jun 13, 2024

Here tests are failing

@halgari
Copy link
Collaborator

halgari commented Jun 13, 2024

Wow, this looks really good! (haven't looked at the code, but the screenshots look awesome.

@captainsandypants
Copy link
Collaborator

This is my bad but when we changed the diagnostics section to "Privacy", I should have also updated the icon.
See updated design. Fair enough if it's too late to change that now though.

@captainsandypants
Copy link
Collaborator

"Minimum severity" diagnostic setting needs updated text.

Figma link

image

…nosticSettings.cs

Co-authored-by: Al <26797547+Al12rs@users.noreply.github.com>
Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

@captainsandypants reviewed in call with me and this should be good to merge

@Al12rs Al12rs merged commit 8eab2e2 into Nexus-Mods:main Jun 17, 2024
7 checks passed
@erri120 erri120 deleted the feat/1370-settings-sections branch June 17, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants