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

feat: right align bottom buttons with forms across app [WD-7985] #594

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

mas-who
Copy link
Collaborator

@mas-who mas-who commented Jan 5, 2024

Done

  • adjusted all creation and edit forms to have max width of 54rem
  • right aligned bottom control buttons to all creation and edit form contents

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • Check all forms with bottom control buttons. Make sure that buttons are right aligned to the form content for all screen sizes and also check if the forms visually look okay.

Screenshots

Example
Screenshot from 2024-01-05 11-25-49

@webteam-app
Copy link

Demo starting at https://lxd-ui-594.demos.haus

@mas-who mas-who changed the title feat: right align bottom buttons with forms across app [WD-7958] feat: right align bottom buttons with forms across app [WD-7985] Jan 5, 2024
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

It looks so much better now with consistent width!

Some ideas to simplify below.

src/components/forms/YamlForm.tsx Outdated Show resolved Hide resolved
src/pages/instances/CreateInstance.tsx Outdated Show resolved Hide resolved
src/pages/storage/forms/StorageVolumeFormMain.tsx Outdated Show resolved Hide resolved
@mas-who mas-who force-pushed the align-form-buttons branch 3 times, most recently from 35d18f1 to 0bef354 Compare January 5, 2024 11:15
@mas-who mas-who requested a review from edlerd January 5, 2024 11:17
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

Found two problems with narrow screens. And a third one, but that might be nasty and can be done as a follow up as well.

  1. The profile selector for instances should not be full width. The up/down/remove buttons should be next to the profiles, see below comparison:

now:
Screenshot from 2024-01-05 12-32-22

before:
Screenshot from 2024-01-05 12-32-11

  1. The secondary form navigation should be scrollable, when on a small screen. To make all the form screens accessible on small screens.

now:
Screenshot from 2024-01-05 12-39-11

before:
Screenshot from 2024-01-05 12-39-19

  1. One last bit, but feel free to ignore it for a future ticket if out of scope. As this was inconsistent before. The scrollbar for the form content is sometimes on the very right of the page and sometimes in the middle at the content end. I think having the scroller in the middle is preferred. It is inconsistent mostly on the "main" form pages, and on the disk selector for instances.

scrollbar at content end:
Screenshot from 2024-01-05 12-45-14

scrollbar at the right
Screenshot from 2024-01-05 12-45-05

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe shorten the name to FormFooter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • For the profile selector, I can revert to spacing from before. The buttons will align to the outer edge of the form so it still looks good. I also noticed that on really narrow screen size, even from before the arrows show above and below the selector. This is due to the arrows having fixed width regardless of the screen size. Maybe we should consider a different design for small screen sizes?
  • For the secondary form navigation, the current behaviour is caused by me removing the overflow css to keep the side navigation static. It's pretty tricky to get buttons aligned perfectly when the side navigation dynamically grows due to it being a scrollable container. I have adjusted css so that the buttons align mostly with the right edge of the form, but it's not perfect. Have a look on the demo server and let me know if it's okay? Alternatively, it might be possible to introduce a empty div right next to the footer container to mimic the behaviour of the side navigation so that the actual footer can be fixed width, but it would introduce non-semantic elements and might be tricky to get the div behave just like the side navigation.
  • For the inconsistent scrollbar issue, I think we should create a ScrollableForm component similar to the ScrollableTable component where it dynamically updates height attributes of the document body. I will create a ticket and circle back on this one later.
  • I think naming it FormFooterLayout indicates clearly that it is a wrapper instead of a self-sufficient component?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • For the profile selector, I can revert to spacing from before. The buttons will align to the outer edge of the form so it still looks good. I also noticed that on really narrow screen size, even from before the arrows show above and below the selector. This is due to the arrows having fixed width regardless of the screen size. Maybe we should consider a different design for small screen sizes?

I think this is fine now.
I can look into it in a follow-up to improve it for smaller screens.

  • For the secondary form navigation, the current behaviour is caused by me removing the overflow css to keep the side navigation static. It's pretty tricky to get buttons aligned perfectly when the side navigation dynamically grows due to it being a scrollable container. I have adjusted css so that the buttons align mostly with the right edge of the form, but it's not perfect. Have a look on the demo server and let me know if it's okay? Alternatively, it might be possible to introduce a empty div right next to the footer container to mimic the behaviour of the side navigation so that the actual footer can be fixed width, but it would introduce non-semantic elements and might be tricky to get the div behave just like the side navigation.

I don't fully understand. It seems to be working fine now, doesn't it? From your description it sounds like scrollbar-gutter: stable might solve the issue as it reserves space for the scrollbar even if it is not rendered.

  • For the inconsistent scrollbar issue, I think we should create a ScrollableForm component similar to the ScrollableTable component where it dynamically updates height attributes of the document body. I will create a ticket and circle back on this one later.

Yeah, that is a good idea, let's keep it as is for now then.

  • I think naming it FormFooterLayout indicates clearly that it is a wrapper instead of a self-sufficient component?

Sounds right.

Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

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

Code and QA looks good, thanks for improving this 👍

- aligned instnace creation and edit forms
- aligned profile creation and edit forms
- aligned project creation and edit forms
- aligned network creation and edit forms
- aligned storage pool creation and edit forms
- aligned storage volume creation and edit forms

Signed-off-by: Mason Hu <mason.hu@canonical.com>
- NOTE: had to remove styling from .form-navigation css class selector to make the side menus on forms not dynamically grow
- aligned buttons for instance creation and edit forms
- aligned buttons for profile creation and edit forms
- aligned buttons for network creation and edit forms
- aligned buttons for project creation and edit forms
- aligned buttons for storage pool creation and edit forms
- aligned buttons for storage volume creation and edit forms

Signed-off-by: Mason Hu <mason.hu@canonical.com>
@mas-who mas-who merged commit 489e126 into canonical:main Jan 5, 2024
6 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 5, 2024
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.

3 participants