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

Improved layout: Set layout push/pull breakpoints to medium for alignment with columns #15459

Closed
wants to merge 1 commit into from
Closed

Improved layout: Set layout push/pull breakpoints to medium for alignment with columns #15459

wants to merge 1 commit into from

Conversation

DragoonAethis
Copy link

Description

The new asset/consumable/user layout introduced in 95a0f3d and a few related commits splits the view into two blocks. On breakpoints XS and SM they're full-width 12-cell containers, while on MD, LG, XL they are 3-cell wide for the button column and 9-cell wide for the details table. Then, extra Bootstrap classes are used to push/pull them so that the button column is on the right.

The push/pull classes were used with the SM breakpoint, while content was still 12-cell wide. This results in the SM breakpoint layout being broken - this commit makes these use the MD breakpoint instead.

output.webm

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Manually checked the CSS change on each affected page. This was changed live on our instance to unblock some people running the desktop layout on their phones.

Checklist:

The new asset/consumable/user layout introduced in 95a0f3d and
a few related commits splits the view into two blocks. On breakpoints
XS and SM they're full-width 12-cell containers, while on MD, LG, XL
they are 3-cell wide for the button column and 9-cell wide for the
details table. Then, extra Bootstrap classes are used to push/pull them
so that the button column is on the right.

The push/pull classes were used with the SM breakpoint, while content
was still 12-cell wide. This results in the SM breakpoint layout being
broken - this commit makes these use the MD breakpoint instead.
@DragoonAethis DragoonAethis requested a review from snipe as a code owner September 7, 2024 10:04
Copy link

welcome bot commented Sep 7, 2024

💖 Thanks for this pull request! 💖

We use semantic commit messages to streamline the release process and easily generate changelogs between versions. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix if it doesn't have one already.

Examples of commit messages with semantic prefixes:

  • Fixed #<issue number>: don't overwrite prevent_default if default wasn't prevented
  • Added #<issue number>: add checkout functionality to assets
  • Improved Asset Checkout: use new notification method for checkout

Things that will help get your PR across the finish line:

  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

Copy link

what-the-diff bot commented Sep 7, 2024

PR Summary

  • Improved Screen Adaptability in Various Files
    We made changes to four files (view-assets.blade.php, view.blade.php, hardware.view.blade.php, users.view.blade.php). We modified certain technical parameters (col-sm-push-9 to col-md-push-9 and col-sm-pull-3 to col-md-pull-3) to make our user interface adapt better to different screen sizes. This ensures a consistent and fluid visual experience for users across different devices.

@DragoonAethis DragoonAethis changed the title Set layout push/pull breakpoints to medium for alignment with columns Improved layout: Set layout push/pull breakpoints to medium for alignment with columns Sep 7, 2024
@snipe
Copy link
Owner

snipe commented Sep 9, 2024

Wasn't this fixed in #15352?

@snipe
Copy link
Owner

snipe commented Sep 9, 2024

@Godmartinz can you pull this branch down and test against your changes that were merged?

@DragoonAethis
Copy link
Author

Ohhh, okay, I did not test this against the current master/develop branch, just the stable one we're running. This probably won't hurt the resource/hardware views if applied, but it also fixes consumable/user views. The extra info-stack classes can be removed.

@snipe
Copy link
Owner

snipe commented Sep 9, 2024

Just not sure that with Godfrey's changes we'd still want to change that from sm to md, since the breakpoints won't line up as expected anymore.

@snipe
Copy link
Owner

snipe commented Sep 17, 2024

@Godmartinz can you weigh in on this one?

@Godmartinz
Copy link
Collaborator

We went with adding an info-stack class and solving it through css, this will give us easier flexibility to sort information later on if we decide to add more information to these pages. But the md change is also a good solution.

@DragoonAethis
Copy link
Author

Okay, looks like this isn't needed then.

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