Add save button to settings UI#7949
Conversation
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> Moved all strings into resources file for localization. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> <!-- Please review the items on the PR checklist before submitting--> * [ ] Closes #xxx * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
|
@leonMSFT or @carlos-zamora, please merge master into settings-ui before we review this PR. |
|
(this branch is based off .. something weird, but merging into feature/settings-ui. thus the conflicts.) |
Kayla was having trouble building the project on her machine (too big). So I did some git magic to put her branch in a state where the Settings UI was its own app. Updated feature/settings-ui and merged that in. This PR is now ready for review! |
|
NOTE: the warning message is currently always displayed. Once we set up data binding, we need to make it so that when a setting is changed, the warning is visible (otherwise, hidden). |
carlos-zamora
added
Area-UserInterface
There was a problem hiding this comment.
This is in response to Carlos's comment - should we also gray-out the Save button if there's nothing to save, also maybe gray-out the reset button if there's nothing changed? (perhaps is a nice little touch but also a bit unnecessary 🤔)
Also I'm curious how the warning TextBlock and the Buttons arrange themselves when you compress the window to a much smaller size, could I get a pic of that?
| <ColumnDefinition Width="*"></ColumnDefinition> | ||
| <ColumnDefinition Width="200"></ColumnDefinition> | ||
| </Grid.ColumnDefinitions> | ||
| <TextBlock x:Uid="Settings_UnsavedSettingsWarning" Grid.Column="0" Foreground="Goldenrod" FontSize="15" VerticalAlignment="Center" HorizontalAlignment="Left" TextAlignment="Left" Margin="30,0,0,0"/> |
There was a problem hiding this comment.
Do we need tooltips for these controls (TextBlock, buttons)?
There was a problem hiding this comment.
I dunno. The text block is its own description.
Should we simply hide the entire save/reset/warning panel when there's nothing to show?
There was a problem hiding this comment.
😆 I now realize asking about a text block tooltip is uh a little redundant, I think the reset button could benefit from a tooltip clarification though (I can see myself thinking it would "reset to default settings"), or maybe it could be called "Discard Changes"?
There was a problem hiding this comment.
We tried mockups with "Discard Changes" and it looked funny. I hope the tooltip I added is good enough?
I agree with the greying out comment, but I figured that would be done programmatically? Unless you want me to have it start that way? Also here's what it looks like at its min width: |
|
yeah we should gray them out or hide them in code. no need for you to do it in this PR 😄 |
| <value>Reset</value> | ||
| </data> | ||
| <data name="Settings_ResetSettingsButton.[using:Windows.UI.Xaml.Controls]ToolTipService.ToolTip" xml:space="preserve"> | ||
| <value>Reset your settings to your previous save.</value> |
There was a problem hiding this comment.
Lol this phrasing sounds like a video game.
Discard may be better :p
4 participants
Added save and reset buttons.
Added warning message for unsaved settings.
References
#1564 - Settings UI Epic