-
Notifications
You must be signed in to change notification settings - Fork 89
feat!: better support header-only card use #3966
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
Draft
brian-smith-tcril
wants to merge
1
commit into
openedx:next
Choose a base branch
from
brian-smith-tcril:center-header-margins
base: next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat!: better support header-only card use #3966
brian-smith-tcril
wants to merge
1
commit into
openedx:next
from
brian-smith-tcril:center-header-margins
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for paragon-openedx-v23 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #3966 +/- ##
=======================================
Coverage 94.37% 94.37%
=======================================
Files 242 242
Lines 4282 4282
Branches 978 1017 +39
=======================================
Hits 4041 4041
+ Misses 237 233 -4
- Partials 4 8 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d63e7a4 to
1315885
Compare
✅ Deploy Preview for paragon-openedx ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
1315885 to
6d546ff
Compare
BREAKING CHANGE:
* Modified `pgn__card-header-content` styles
* `margin-top` changed:
* Before: 2x `var(--pgn-spacing-spacer-base)`
* After: 1x `var(--pgn-spacing-spacer-base)`
BREAKING CHANGE:
* Modified `pgn__card-header-actions` styles
* Children now centered
* `display: flex` added
* `align-items: center` added
BREAKING CHANGE:
* `pgn__card-wrapper-image-cap.vertical` modified
* Bottom margin conditionally added when logo image is used. This replicates the spacing previously baked into `pgn__card-header-content`
* `margin-bottom: var(--pgn-spacing-spacer-base)` added when logo image used
6d546ff to
d396673
Compare
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGE:
pgn__card-header-contentstylesmargin-topchanged:var(--pgn-spacing-spacer-base)var(--pgn-spacing-spacer-base)BREAKING CHANGE:
pgn__card-header-actionsstylesdisplay: flexaddedalign-items: centeraddedBREAKING CHANGE:
pgn__card-wrapper-image-cap.verticalmodifiedpgn__card-header-contentmargin-bottom: var(--pgn-spacing-spacer-base)added when logo image usedDescription
frontend-app-authoringuses cards in a header-only mannerThe current styling for that MFE is accomplished via custom style overrides
This is definitely not an ideal way to accomplish this, however, removing this custom styling leads to less-than-ideal styles in the MFE
This can be made slightly better by adding an empty
Card.FooterBut it still looks "off."
Replicating the "off" styling on the Paragon docs site
It already looks kind of "off" here
This can also be addressed somewhat by adding
Card.Footerto the examples, but again, still looks "off"What this PR does:
Card.Footers to theHeaderexamples on the docs sitecard-header-contentto match the top margin oncard-header-actionscard-header-actionsto usedisplay: flexandalign-items: centerConditionally adds a new
with-logoclass when aCard.ImageCaphas a logo to ensure the header maintains the current spacingWith
with-logoWithout
with-logoDeploy Preview
https://deploy-preview-3966--paragon-openedx-v23.netlify.app/components/card/#header
Merge Checklist
exampleapp?Post-merge Checklist