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

Added desktop directions (right to left, left to right, top to bottom, bottom to top) #88

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

aletail
Copy link
Member

@aletail aletail commented Mar 19, 2024

Summary

Added desktop directions (right to left, left to right, top to bottom, bottom to top)

Testing Steps

  • Test with any site
  • On a container (If a newer site) test the Layout -> Direction settings, confirm these work for Desktop

Issues/Concerns

  • This originally came up when updating flexcut, their "cart" page uses a direction class (The side bar should appear on the right side, not the left)

Git Flow

  • DO NOT delete "release/*" or "hotfix/*" branches after merging a PR. These are used to publish the next release, and they are deleted automatically.
  • "Squash and merge" is good on "feature/*" into "develop"
  • "Create a merge commit" is good on "release/*" or "hotfix/*" into "main"
  • With npm repositories, the version number must be incremented manually, before merging the release.

@aletail aletail requested a review from tiller1010 March 19, 2024 17:59
@aletail aletail added the patch label Mar 19, 2024
Copy link
Member

@tiller1010 tiller1010 left a comment

Choose a reason for hiding this comment

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

  • The direction is only set in @media screen and (min-width: 1366px), so it reverts back to the initial direction if there isn't a tablet- class too.

With npm repositories, the version number must be incremented manually, before merging the release.

  • We'll need to update package.json to 2.1.14

@aletail aletail requested a review from tiller1010 March 20, 2024 16:50
&.desktop-btt{
flex-direction: column-reverse;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

There are still duplicate rules inside of the media query.
We could also still make use of breakpointDesktop, by simply moving those styles outside of the media query, but still within the @each

/*
  DESKTOP BREAKPOINTS
  Loop through each desktop defined breakpoint
*/
@each $label, $breakpoint in getThemeProperty(breakpointDesktop){

  /* Flex Container Direction */
  .flex-container{
    &.#{$label}-ltr{
      flex-direction: row;
    }
    &.#{$label}-rtl{
      flex-direction: row-reverse;
    }
    &.#{$label}-ttb{
      flex-direction: column;
    }
    &.#{$label}-btt{
      flex-direction: column-reverse;
    }
  }

  @media screen and (min-width: $breakpoint) {
    .#{$label}-half{
      width: 0.5%;
      min-height: 1px;
    }
    @for $i from 1 through 100 {
      .#{$label}-#{$i} {
        width: round(percentage(math.div($i, 100)));
      }
    }
    .#{$label}-auto{
      width: auto;
    }
    .#{$label}-hide{
      display: none;
    }
    .#{$label}-show{
      display: inherit;
    }
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

There are still duplicate rules inside of the media query.

Right, not sure how to get around this. The "desktop-" rules outside of the mediaquery act as a default/fall back mostly - we could rename those to "default-" ....but that would be a major change and require more class tags to be used in the html. If we are looking to optimize this and remove the duplicate rules I think we should handle in a separate PR

We could also still make use of breakpointDesktop, by simply moving those styles outside of the media query, but still within the @each

But this would add those for each breakpoint added to the desktop list and they would exist outside of their related mediaquery

@aletail aletail requested a review from tiller1010 March 22, 2024 12:25
Copy link
Member

@tiller1010 tiller1010 left a comment

Choose a reason for hiding this comment

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

The "desktop-" rules outside of the mediaquery act as a default/fall back mostly...
...this would add those for each breakpoint added to the desktop list and they would exist outside of their related mediaquery

This makes sense. Everything looks good then!

@aletail aletail merged commit 08a14d3 into main Mar 26, 2024
1 check passed
@github-actions github-actions bot deleted the hotfix/2.1.14 branch March 26, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants