-
Notifications
You must be signed in to change notification settings - Fork 38
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(card): css changes and data attrs #2398
Conversation
🦋 Changeset detectedLatest commit: e4ebd5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Preview deployments for this pull request: 📖 Storybook |
Coverage Report
File Coverage
|
Preview deploymentsTheme |
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.
nice work 👍 La inn et par comments, men er kanskje ikke veldig viktig - vi må se litt mer på Card i forbindelse med UU; kan være card som lenke ikke nødvendigvis skal være en komponent, men heller ha lenken alltid inni og utvide klikkflate med onClick
(se gjerne mer på https://adrianroselli.com/2020/02/block-links-cards-clickable-regions-etc.html) - det må vi diskutere i gruppa i forbindelse med #2355
.ds-card__header h4, | ||
.ds-card__header h5, | ||
.ds-card__header h6 { | ||
color: inherit; |
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.
Kanskje noen jeg ikke har fått med meg - hvorfor trenger vi å sette color: inherit
på disse? 😇
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.
Sjå kommentar under for svar. #2398 (comment)
& h3, | ||
& h4, | ||
& h5, | ||
& h6 { |
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.
Skulle vi heller gjort & :is(h1,h2,h3,h4,h5,h6)
? 😊
og hvorfor setter vi color
på heading - arver ikke den farge? 😇
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.
Me setter farge slik at den blir konsekvent rett i forhold til fargen kortet har. Jo den arver farge, men vår Heading
komponent setter også farge til --ds-color-neutral-text-default
, som då blir satt om me ikkje setter farge her.
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.
Ah, hvorfor setter Heading farge? Kan det være litt vanskelig å sette en heading på en flate også arver den ikke currentcolor? Burde vi snakke med design om dette? :)
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.
Gjerne ta det med design, føler ikkje det er eit valg me i kode skal ta.
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.
Tenker det har litt med kode å gjøre fordi det setter føringer for å kunne gjenbrukbare komponenter inni flater - jeg poster en diskusjon om dette her: https://designsystemet.slack.com/archives/C05U1MNKYCX/p1726480465469179
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.
Okei. Avventer endringa til me får svar der.
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.
Tenker vi kan endre på det senere - kanskje bare merge denne nå først så lager jeg eget issue med color: inherit på typografi-komponenter
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to next, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `next` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `next`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @digdir/[email protected] ### Patch Changes - Pagination: Use data attrs instead of class names ([#2395](#2395)) - Badge: Style using css attributes ([#2391](#2391)) - TableHeaderCell: Remove `sortable` prop, `sort` now handles this ([#2393](#2393)) - dropdownmenu: Style using data attributes ([#2387](#2387)) - Chip: Text color is now `accent` ([#2371](#2371)) - List: Remove `List.Root` and `List.Heading`, which changes API ([#2348](#2348)) - Alert, Avatar, Button, Divider, Link: Use data-attributes for variant, size and color and move icons to CSS ([#2313](#2313)) - Box: Remove component ([#2372](#2372)) - Popover: ([#2369](#2369)) - Rename `<Popover.Root>` to `<Popover.Context>` - use Popover API, allowing `<Popover>` to be used without `Popover.Context` - Remove `portal` prop - Tooltip: Only expose background css variable ([#2389](#2389)) - Switch: don't show check when not checked in readonly ([#2377](#2377)) - Select: Rename from `NativeSelect` ([#2404](#2404)) - Accordion: Now uses details and summary HTML elements ([`5d1c5062b526e6829c322ce66c6df08568bb9f63`](5d1c506)) - Spinner: Style using data attributes ([#2390](#2390)) - Avatar: new component ([#2312](#2312)) - Tag: Make neutral default color in CSS ([#2397](#2397)) - Card: Use data attrs ([#2398](#2398)) ## @digdir/[email protected] ### Patch Changes - Pagination: Use data attrs instead of class names ([#2395](#2395)) - Badge: Style using css attributes ([#2391](#2391)) - TableHeaderCell: Remove `sortable` prop, `sort` now handles this ([#2393](#2393)) - dropdownmenu: Style using data attributes ([#2387](#2387)) - List: Remove `List.Root` and `List.Heading`, which changes API ([#2348](#2348)) - Alert, Avatar, Button, Divider, Link: Use data-attributes for variant, size and color and move icons to CSS ([#2313](#2313)) - Box: Remove component ([#2372](#2372)) - Popover: ([#2369](#2369)) - Rename `<Popover.Root>` to `<Popover.Context>` - use Popover API, allowing `<Popover>` to be used without `Popover.Context` - Remove `portal` prop - Select: Rename from `NativeSelect` ([#2404](#2404)) - Table: Set sort button type to prevent form submit ([#2402](#2402)) - Heading: default level is now 2 ([#2378](#2378)) - Select: ([#2415](#2415)) - Add Select.Option and Select.Optgroup compond components - Remove `multiple` prop - Accordion: Now uses details and summary HTML elements ([`5d1c5062b526e6829c322ce66c6df08568bb9f63`](5d1c506)) - Spinner: Style using data attributes ([#2390](#2390)) - Avatar: new component ([#2312](#2312)) - Tag: Make neutral default color in CSS ([#2397](#2397)) - Card: Use data attrs ([#2398](#2398)) - Combobox: fix virtual combobox having large gap between items ([#2376](#2376)) ## @digdir/[email protected] ## @digdir/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
part of #2295