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

[Hosts] Improve Resizing behavior #32788

Merged
merged 3 commits into from
May 31, 2024
Merged

Conversation

htcfreek
Copy link
Collaborator

@htcfreek htcfreek commented May 9, 2024

Summary of the Pull Request

The old ui definition causes the host names column to hid. Now it is not hiding anymore and the Address column has a smaller min width.

Resizing

Before:
Screenshot_20240509-205052_GitHub.jpg

After:
hosts_ui_resizing

Alignment on small window with correct resizing behavior

Before:
image

After:
image

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • Set min width for the first two columns.
  • Updated the min width of he window that the last columns not hiding on resizing.
  • Set fixed column width for the icons of the entries to have them vertical aligned.
  • Fixed the bug that the icons and buttons can get misaligned on resizing.
    Screenshot_20240509-204221_GitHub.jpg

Validation Steps Performed

Manual tests.

@htcfreek htcfreek marked this pull request as draft May 9, 2024 09:40
@htcfreek
Copy link
Collaborator Author

htcfreek commented May 9, 2024

@davidegiacometti
If you like to have the ping & duplicate icons centered instead of vertical aligned please tell me. Then we can change the width properties back to Auto.

@htcfreek htcfreek marked this pull request as ready for review May 9, 2024 10:45
@htcfreek htcfreek self-assigned this May 9, 2024
Copy link
Collaborator

@davidegiacometti davidegiacometti left a comment

Choose a reason for hiding this comment

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

Hi @htcfreek
Thanks for looking into this!
This PR reminded me #28483
The idea was to get rid of the fixed width of the address column using the DataTable from the CommunityToolkit Labs package. I gave a try in the past but the control wasn't mature enough to be used (don't know what's the current state).

Don't like the IPv6 being wrapped but I think this is an improvement.

src/modules/Hosts/HostsUILib/HostsMainPage.xaml Outdated Show resolved Hide resolved
src/modules/Hosts/HostsUILib/HostsMainPage.xaml Outdated Show resolved Hide resolved
@htcfreek
Copy link
Collaborator Author

Don't like the IPv6 being wrapped but I think this is an improvement.

@davidegiacometti
It is wrapped because I made the column size smaller as requested in the issue. Making ut bigger again would require a higher min window size, because resizing the window otherwise hides the buttons.

@htcfreek
Copy link
Collaborator Author

htcfreek commented May 10, 2024

@davidegiacometti
I made some changes based on your feedback. This is how it looks now. Are you okay with it?
image

The windows shows in the default open size and is not resized.

@davidegiacometti
Copy link
Collaborator

Looks better! I will test and review the changes.

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the contribution!

@htcfreek
Copy link
Collaborator Author

@jaimecbernardo
You can merge this. I don't have the permission for merging.

@jaimecbernardo jaimecbernardo merged commit 92e8b06 into microsoft:main May 31, 2024
10 checks passed
@htcfreek htcfreek deleted the PT_HostsUi branch May 31, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants