-
Notifications
You must be signed in to change notification settings - Fork 0
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/tup 637: Migrate c-card support list to core-styles #250
Conversation
Note:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, works. Architecturally, off. But we'll get there.
Blame my haphazard use of Fractal naming (to understand, see "Fractal vs. BEM").
Please add the new styles to c-card.css
, not c-card-list.css
. You may continue to use _c-card--list.hbs
for the name of the reusable demo template.1
Why?
These styles are for any list in a card (any card), not a list-specific "modifier"* of a card.
* A "modifier", as in the "M" of BEM. CSS Style Guide - Naming Conventions - BEM.
Fractal vs. BEM
In Fractal, to create a "variant" (a.k.a. BEM class modifier), one writes c-card--var-aka-mod
. I used that same syntax for creating Handlebar templates to be included into a demo template i.e. _c-card--one-plain.hbs
(the underscore hides the template from the demo navigation). Should I? Must I? I don't know. Feedback welcome.
Footnotes
-
If feedback and discussion lead to new standard, we can update after these after this PR. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm taking a little more liberty on this one, to get it done sooner, so we can test in bulk this week on dev. But I let you review the change.
@@ -185,4 +185,20 @@ | |||
/* SEE: https://www.tacc.utexas.edu/education/k-12-students/high-school-camps/gencyber/ */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 184-187:
This has popped up twice (once in this branch, and second in another: 254)
I'm not sure how to handle this, and I'm not sure deleting it is the right choice. In 254, I did delete, but here, I just moved it to it's original spot. Let me know what you think @wesleyboar when you review this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Delete please.
- The effect of the style is negligible.
- The
<h4>
is demo'd by c-card--image-top for contact card.
Overview
Migrate c-card list support to core-styles
Outdated Notices
Related
Changes
Importing c-card list support from tup-ui to core-styles
Testing
Verify Card styles here
UI