Convert Card component to Typescript #806
Conversation
Bundle ReportChanges will decrease total bundle size by 1.35kB (-0.01%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @open-formulieren/sdk-OpenForms-umdAssets Changed:
Files in
view changes for bundle: @open-formulieren/sdk-esmAssets Changed:
Files in
Files in
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
==========================================
- Coverage 83.94% 83.17% -0.78%
==========================================
Files 232 232
Lines 4623 4613 -10
Branches 1176 1169 -7
==========================================
- Hits 3881 3837 -44
- Misses 712 747 +35
+ Partials 30 29 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sergei-maertens
force-pushed
the
chore/445-convert-more-to-ts
branch
from
41cac4c to
181c898
Compare
sergei-maertens
force-pushed
the
chore/445-convert-more-to-ts
branch
2 times, most recently
from
4fc05af to
fcbbce6
Compare
Depends on #806, now we have type errors on the CardTitle usage. All in all a pretty straight forward conversion after most of the code got deleted in the earlier refactor.
Depends on #806, now we have type errors on the CardTitle usage. All in all a pretty straight forward conversion after most of the code got deleted in the earlier refactor.
... and cleaned up its usage along the way. The CardTitle component is used in either one of two ways: * implicitly, through the Card where it's basically _the_ title and should be displayed as h1 element * explicitly, in which case it was always being rendered as an h2 element So, the default heading level is now 2 for the low-level component, while using it implicitly through Card ensures that it's level 1, matching the current behaviour but cleaning up the code quite a bit. This cleanup removes the need for polymorphic component props to support multiple HTML elements - our codebase does not use anythign other than heading elements at the moment (and those landmarks are important for a11y). Next, the Card component itself. The modifiers list is gone in favour of explicit (boolean) props that set/determine the modifiers under the hood, allowing proper combination and usage tracking while being able to document their behaviour individually. Usages have been updated accordingly, but let's wait for Chromatic to confirm nothing broke. The caption did not appear to be used anywhere as Card prop, so that's been removed.
sergei-maertens
force-pushed
the
chore/445-convert-more-to-ts
branch
from
fcbbce6 to
dfffb7b
Compare
Depends on #806, now we have type errors on the CardTitle usage. All in all a pretty straight forward conversion after most of the code got deleted in the earlier refactor.
... and cleaned up its usage along the way.
The CardTitle component is used in either one of two ways:
So, the default heading level is now 2 for the low-level component, while using it implicitly through Card ensures that it's level 1, matching the current behaviour but cleaning up the code quite a bit. This cleanup removes the need for polymorphic component props to support multiple HTML elements - our codebase does not use anythign other than heading elements at the moment (and those landmarks are important for a11y).
Next, the Card component itself. The modifiers list is gone in favour of explicit (boolean) props that set/determine the modifiers under the hood, allowing proper combination and usage tracking while being able to document their behaviour individually. Usages have been updated accordingly, but let's wait for Chromatic to confirm nothing broke.
The caption did not appear to be used anywhere as Card prop, so that's been removed.