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

ContentDialog visual update #3926

Merged
merged 6 commits into from
Jan 14, 2021
Merged

ContentDialog visual update #3926

merged 6 commits into from
Jan 14, 2021

Conversation

tashatitova
Copy link
Contributor

@tashatitova tashatitova commented Jan 12, 2021

Updating the brushes, animations, layout logic

3-button-dark

3-button-hc

3-button-light

2-button-dark

2-button-hc

2-button-light

@tashatitova tashatitova self-assigned this Jan 12, 2021
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jan 12, 2021
@mdtauk
Copy link
Contributor

mdtauk commented Jan 12, 2021

Could you post a Light and Dark image showing the new visual style?

Also what do your layout logic changes mean for developers?

@tashatitova
Copy link
Contributor Author

Could you post a Light and Dark image showing the new visual style?

Also what do your layout logic changes mean for developers?

Added screenshots.

The logic changes nothing for the developers. Just makes 2-3 column layout logic cleaner with additional fixed columns for spacing, which is stored as a resource and can be easily updated in one spot should we need this later.

@mdtauk
Copy link
Contributor

mdtauk commented Jan 12, 2021

Could you post a Light and Dark image showing the new visual style?
Also what do your layout logic changes mean for developers?

Added screenshots.

The logic changes nothing for the developers. Just makes 2-3 column layout logic cleaner with additional fixed columns for spacing, which is stored as a resource and can be easily updated in one spot should we need this later.

Thank you for the visuals, that bottom darker bar with the buttons makes a huge difference (design and aesthetic wise)
Has some stuff in common with the Win32 Dialogs too.

image
image

@StephenLPeters StephenLPeters added area-Dialogs area-Styling team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jan 13, 2021
@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

</Grid>
</ScrollViewer>
<Grid x:Name="CommandSpace" Grid.Row="1" HorizontalAlignment="Stretch" VerticalAlignment="Bottom" XYFocusKeyboardNavigation="Enabled" Margin="{ThemeResource ContentDialogCommandSpaceMargin}">
<Grid x:Name="CommandSpace"
Copy link
Contributor

Choose a reason for hiding this comment

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

Format nit: move x:Name to new line

@harvinders
Copy link

Currently increasing the width of the dialog also increases the width of the button. Say, at the width 1000 for content dialog, a single button looks odd (too stretched). Are we going to look into that too here?

@tashatitova
Copy link
Contributor Author

Currently increasing the width of the dialog also increases the width of the button. Say, at the width 1000 for content dialog, a single button looks odd (too stretched). Are we going to look into that too here?

This will be a separate bug as it's not a regression. @stmoy is there an open issue at the moment?

@tashatitova
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdtauk
Copy link
Contributor

mdtauk commented Jan 14, 2021

Currently increasing the width of the dialog also increases the width of the button. Say, at the width 1000 for content dialog, a single button looks odd (too stretched). Are we going to look into that too here?

This will be a separate bug as it's not a regression. @stmoy is there an open issue at the moment?

This could be done as a behaviour on the ContentDialog. To have the bottom buttons stretch to fill the width, or to align to the right (or the left with Right To Left flow)

@stmoy
Copy link
Contributor

stmoy commented Jan 14, 2021

Currently increasing the width of the dialog also increases the width of the button. Say, at the width 1000 for content dialog, a single button looks odd (too stretched). Are we going to look into that too here?

This will be a separate bug as it's not a regression. @stmoy is there an open issue at the moment?

This is "by design" so no bug is filed - the main reason is that ContentDialogs are expected to always have at least two buttons link. In that context, it's not a huge deal that a single button dialog looks strange because there shouldn't be any single button dialogs.

That said, @mdtauk posted a reasonable one-button dialog so it might be reasonable to reassess the guidance. This should be reassessed separately from this PR though.

@mdtauk
Copy link
Contributor

mdtauk commented Jan 14, 2021

Currently increasing the width of the dialog also increases the width of the button. Say, at the width 1000 for content dialog, a single button looks odd (too stretched). Are we going to look into that too here?

This will be a separate bug as it's not a regression. @stmoy is there an open issue at the moment?

This is "by design" so no bug is filed - the main reason is that ContentDialogs are expected to always have at least two buttons link. In that context, it's not a huge deal that a single button dialog looks strange because there shouldn't be any single button dialogs.

That said, @mdtauk posted a reasonable one-button dialog so it might be reasonable to reassess the guidance. This should be reassessed separately from this PR though.

What about a single button, but also beside it, a Hyperlink Button?
image

@tashatitova tashatitova merged commit 67bb967 into master Jan 14, 2021
@tashatitova tashatitova deleted the user/tatito/dialog branch January 14, 2021 21:37
@mdtauk
Copy link
Contributor

mdtauk commented Jan 15, 2021

@tashatitova There is possibly an issue with the new ContentDialog design. The bottom part of the dialog, with the buttons in, seems to darken whilst the dialog transitions on and off screen.

image
Stills from a screen recording

The final frames of the dialog transition, appears jarring.

Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Jul 10, 2022
Kinnara added a commit to Kinnara/ModernWpf that referenced this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants