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

Adjust Settings window so that content expands as it resizes #193

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

pingzing
Copy link
Contributor

Minor fix, unrelated to my other PR. Allows the Settings window's content to expand vertically, along with the window.

Before After
before after

Note: the Height set in SettingsWindow.axaml gets ignored because SizeToContent is set to SizeToContent.Height. If we want the window to open up to the height of 500 as set, we'll need to remove that from the constructor.

Comment on lines -162 to -166
<RowDefinition Height="Auto" />
<RowDefinition Height="Auto" />
<RowDefinition Height="25" />
<RowDefinition Height="50" />
<RowDefinition Height="Auto" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed, as the stuff inside the ScrollViewer is no longer part of this Grid.

<Border Grid.Row="0" BorderBrush="{DynamicResource HighlightBrush}" BorderThickness="0 0 0 2" Grid.ColumnSpan="2">
<Menu Name="menuBar" KeyboardNavigation.TabNavigation="None">
<Image Source="/Assets/icon.ico" Stretch="None"/>
<TextBlock Text="{i18n:Translate ui.window.settings.name}" Margin="-10 0 0 0" VerticalAlignment="Center" HorizontalAlignment="Left" Foreground="{DynamicResource ThemeForegroundBrush}" />
</Menu>
</Border>
<Menu Name="windowMenu" IsVisible="{Binding ShowMainMenu}" HorizontalAlignment="Right" KeyboardNavigation.TabNavigation="None" Grid.Column="1">
<Menu Name="windowMenu" Grid.Row="0" IsVisible="{Binding ShowMainMenu}" HorizontalAlignment="Right" KeyboardNavigation.TabNavigation="None" Grid.Column="1">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not strictly necessary, because any item without a Grid.Row set is implicitly 0, but this makes things more consistent.

<Button Name="exitButton" i:Attached.Icon="mdi-close" Margin="0 0 -10 0" Foreground="{DynamicResource ThemeForegroundBrush}" Background="Transparent"/>
</Menu>

<ScrollViewer AllowAutoHide="True" Height="350" Grid.Row="1" Grid.ColumnSpan="2">
<ScrollViewer AllowAutoHide="True" Grid.Row="1" Grid.ColumnSpan="2">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the explicit height, which will allow the ScrollViewer to expand to fit its container.

<StackPanel>
<StackPanel Grid.Row="1" Grid.ColumnSpan="2" Margin="10 10 0 0">
<StackPanel Margin="10 10 0 0">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This, and all of the other children inside the StackPanel are no longer part of a Grid (and never were, actually), so don't need this.

<Button Name="cancelButton" IsCancel="True" ToolTip.Tip="{Binding ToolTip_Cancel}" i:Attached.Icon="mdi-cancel" Margin="0 0 15 0" BorderBrush="{DynamicResource HighlightBrush}" Foreground="Red" Background="Transparent" HorizontalAlignment="Right"/>
</DockPanel>
<Border Grid.Row="2" Grid.ColumnSpan="2" BorderBrush="{DynamicResource HighlightBrush}" BorderThickness="0 0 0 2" Height="4"/>
<Grid Grid.Row="3" Grid.ColumnSpan="2" RowDefinitions="Auto, Auto" ColumnDefinitions="Auto, Auto" HorizontalAlignment="Right" Margin="0 15 0 15">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Little container grid, to hold the two buttons. This probably could have just been a StackPanel with a Horizontal Orientation too, but ¯\_(ツ)_/¯

@Floogen Floogen added the being looked at Currently being investigated label Apr 22, 2024
@Floogen
Copy link
Owner

Floogen commented Apr 22, 2024

This is great! Thank you for getting that menu fixed up.

I made a minor adjustment to have the height of SettingsWindow based on the parent window and removed the SizeToContent.Height restriction.

If that change looks good to you, I will merge it over.

@pingzing
Copy link
Contributor Author

pingzing commented Apr 22, 2024

I'd just pass in the desired height rather than the whole Window. Separation of concerns and all that. But otherwise, looks fine 👍

EDIT: oh, dang and you might want to remove the explicit Height in the XAML file too. Otherwise in three months you'll be like "...why am I setting the height twice?"

@Floogen Floogen merged commit 3094ef1 into Floogen:development Apr 22, 2024
@Floogen
Copy link
Owner

Floogen commented Apr 22, 2024

Good idea on both points! I have added those changes and merged it over.

Thank you for the pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
being looked at Currently being investigated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants