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

css[Op#48413] Add explicit margin bottom to project storages danger zoneul using primer css #13301

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

akabiru
Copy link
Member

@akabiru akabiru commented Jul 31, 2023

https://community.openproject.org/work_packages/48413

🚧 Bug: Missing Margin Bottom

broken-spacing-danger-zone

Since the Primer release in #13126 global_styles/vendor/foundation-apps/scss/components/_typography.scss was removed which defined a margin-bottom for ul elements.

// DEPRECATED: global_styles/vendor/foundation-apps/scss/components/_typography.scss
  /* Lists */
  ul,
  ol,
  dl {
    margin-bottom: $list-margin-bottom;
    // ....
  }

Hence update to use PrimerCSS Margin Directional utility

Screenshot 2023-07-31 at 2 53 09 PM

`global_styles/vendor/foundation-apps/scss/components/_typography.scss`
defined `ul` elements with a default `margin-bottom`

```css
  /* Lists */
  ul,
  ol,
  dl {
    font-size: $list-font-size;
    line-height: $list-line-height;
    margin-bottom: $list-margin-bottom;
    list-style-position: $list-style-position;
    font-family: $list-font-family;
  }
```

Since the Primer release, this has since been removed. Hence updating to
using PrimerCSS [Margin Directional](https://primer.style/css/storybook/?path=/story/utilities-margin--directional) utility

https://community.openproject.org/work_packages/48413
@akabiru akabiru marked this pull request as ready for review July 31, 2023 12:04
@akabiru akabiru requested review from HDinger and a team July 31, 2023 12:04
@akabiru akabiru changed the title css[Op#48413] Add explicit margin bottom to ul from primer update css[Op#48413] Add explicit margin bottom to project setting danger zoneul using primer css Jul 31, 2023
@akabiru akabiru changed the title css[Op#48413] Add explicit margin bottom to project setting danger zoneul using primer css css[Op#48413] Add explicit margin bottom to project storages danger zoneul using primer css Jul 31, 2023
Copy link
Contributor

@HDinger HDinger left a comment

Choose a reason for hiding this comment

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

Thanks a lot, for the detailed description I love that! It makes the review so much easier ☺️
The solution is fine as well 👍

@HDinger HDinger merged commit fd9e442 into dev Jul 31, 2023
5 checks passed
@HDinger HDinger deleted the bug/48413-danger-zone-margin-bottom-list branch July 31, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants