Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: XAML refactoring #29785
Settings: XAML refactoring #29785
Changes from 37 commits
8cb7648
2f4c654
c55f7ee
d64fff6
e77f046
d1bd00d
a8a1bee
50f0d42
456f71f
433529e
bedc436
36856bd
0be5031
6c27ff8
2fd381e
e1e817f
99112f0
207ed6e
28e01f2
8c54dca
ac983e3
90ad0cb
5699a44
a207c58
bb25081
ba51454
fae4db9
3363b97
e11bbd3
a083069
68d7b99
4010e44
ebce33c
fd684f4
59e5e45
d3ac12a
f7ca401
4848fd7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe increase the spacing between the sections a bit.. either by increasing the spacing (red) or decrease (blue) the bottom part of each section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I knew you would mention this! 😄 My choice is based on:
https://learn.microsoft.com/nl-nl/windows/apps/design/basics/content-basics#text--hierarchy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the spacing is fine, but there is no hierarchy anymore. The title of a 'section' should provide more spacing so you know that the title/description belongs to the legend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a Style would be better, then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just adding an additional top-margin to the titles/descriptions should work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The information shown in this pop-up are sections, right? There needs to be more vertical spacing between sections vs. the title/label and content of a section. This helps to create a visual hierarchy. I don't believe the guidelines you mentioned describe the spacing between sections?
Here's an example from the Fluent Windows Visual Library in Figma, which should be considered as our redlines for things. See how the spacing between the different sections is slightly higher (red) than the spacing between the header of the textbox (blue)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the design library, the spacing between sections (red) is 24px and spacing between header/child (blue) is 12px. Might be slightly different in XAML with the rendering of the TextBlock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let's take one step back: Do we consider the legend as a control, or as text? That's a big difference. MS Learn actually says to use 8px between control and its header, but looks clear on the 12px between text.
Also: I don't know Figma and I don't understand why you keep bringing that up. I just want to use Content design basics for Windows apps. If anything on MS Learn is outdated, that info should be updated. Otherwise I'm sticking to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs are outdated here and there (as you can see your article is from 2021) - I think for this topic specifically, they are still correct.
In Figma it's 12px, but could very well be it's 8px in epx. It's a detail, the point is that the current spacing between the headers of the sections does not provide the correct spacing.
For redlines and inspiration, the Windows Design team recommends to use the Fluent Windows Design Library - it's listed on the
Toolkits and libraries
page:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it 12 and 8.