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: make settings width consistent #5233

Conversation

Jay-Topher
Copy link
Contributor

@Jay-Topher Jay-Topher commented Dec 7, 2023

Done

  • Upgraded @canonical/maas-react-components
  • made settings views with forms half width and aligned action buttons to the right.
  • made views with tables to occupy the full width available
  • Added titles to all settings pages

QA steps

  • Visit /settings and all its sub-routes
  • Ensure that all pages under settings have a title same as the side nav link title
  • Ensure width of form and tabular views respectively are consistent

Fixes

Fixes: MAASENG-2403

Screenshots

Before

image

After

image

Notes

@webteam-app
Copy link

Demo starting at https://maas-ui-5233.demos.haus

Copy link
Contributor

@ndv99 ndv99 left a comment

Choose a reason for hiding this comment

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

I think this looks good so far, just a few comments below

src/scss/index.scss Outdated Show resolved Hide resolved
src/scss/index.scss Outdated Show resolved Hide resolved
src/app/settings/views/Configuration/General/General.tsx Outdated Show resolved Hide resolved
@Jay-Topher Jay-Topher force-pushed the MAASENG-2403-make-settings-width-consistent branch 3 times, most recently from 301951c to 63495c6 Compare December 8, 2023 14:40
@Jay-Topher Jay-Topher marked this pull request as ready for review December 8, 2023 15:11
Copy link
Contributor

@ndv99 ndv99 left a comment

Choose a reason for hiding this comment

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

/settings/configuration/general needs some changes IMO - there's definitely not enough space for the theme colour labels in this layout. I'd suggest modifying this so that the default layout for these is two rows of five.

image

@Jay-Topher Jay-Topher force-pushed the MAASENG-2403-make-settings-width-consistent branch from 63495c6 to ca10e53 Compare December 11, 2023 08:01
@Jay-Topher Jay-Topher force-pushed the MAASENG-2403-make-settings-width-consistent branch from ca10e53 to 1162f64 Compare December 11, 2023 08:03
@petermakowski
Copy link
Contributor

Google Chrome screenshot 001200@2x

@Jay-Topher Headings should be aligned. Please remove the <section class="p-strip is-shallow"> wrapper that's responsible for the additional padding. In case there are still some minor discrepancies, I'll resolve them in maas-react-components.

@Jay-Topher Jay-Topher force-pushed the MAASENG-2403-make-settings-width-consistent branch from 1162f64 to 51da80e Compare December 11, 2023 11:51
@Jay-Topher Jay-Topher force-pushed the MAASENG-2403-make-settings-width-consistent branch from 51da80e to 045002b Compare December 11, 2023 12:12
Copy link
Contributor

@ndv99 ndv99 left a comment

Choose a reason for hiding this comment

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

I'm happy with this personally, everything looks good to me. Nice one mate!

@petermakowski
Copy link
Contributor

LGTM

@petermakowski petermakowski changed the title chore: make settings width consistent feat: make settings width consistent Dec 12, 2023
@petermakowski
Copy link
Contributor

@Jay-Topher Changed prefix to feat: as this affects the end user.

@petermakowski petermakowski enabled auto-merge (squash) December 12, 2023 07:38
@petermakowski petermakowski merged commit e53d51d into canonical:main Dec 12, 2023
5 of 6 checks passed
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.

4 participants