-
Notifications
You must be signed in to change notification settings - Fork 6
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
[#2480] Add NLDS paragraph components #1215
Conversation
4f0d754
to
fffc00d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1215 +/- ##
========================================
Coverage 95.22% 95.22%
========================================
Files 968 968
Lines 35249 35249
========================================
Hits 33566 33566
Misses 1683 1683 ☔ View full report in Codecov by Sentry. |
702c0f2
to
29aaf04
Compare
@Bartvaderkin and @pi-sigma : So here is another fun big changes NLDS thing. This time I'm using the proper way to style 'modifiers' like |
29aaf04
to
63f119f
Compare
src/open_inwoner/components/templates/components/Card/RenderCard.html
Outdated
Show resolved
Hide resolved
src/open_inwoner/components/templates/components/Messages/Messages.html
Outdated
Show resolved
Hide resolved
Adding 1.17.2 tag but considering the amount of work left I'm not sure if that is applicable |
e87ff02
to
1f02c18
Compare
e528595
to
3906b21
Compare
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.
Rebasing on develop
will create a merge conflict due to pdc/tests/test_views.py
. I would fix it myself, but rebasing also marked a bunch of other files as having changed, so we should wait until Monday. Will take this up with Jiro first thing in the morning.
3906b21
to
674573f
Compare
issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/2480
paragraph modifiers that Utrecht doesn't have are:
--compact
,--small
,--muted
,--centered
This means we will have to make "
--oip
" modifiers with our own designtokens, so that municipalities that do not wish to use our design-tokens can use either their own or add custom-styling themselves.HTML example of the proper way of working:
This is what it looked/looks like in the past/legacy code:
<p class="p p--small">…..
But now this should become:
<p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-small">.....
We will not yet be implementing this for ALL
<p>
tags because we have them in our own components that do not exist in NLDS at all. (Like the paragraphs inside the 'Mijn profiel' tabled sections for example).Also: in future I would love to completely remove all "legacy styling" that's meant to keep things backwards compatible for now. Plus it seems we have unused code, like
location-card-list
which might be safe to delete but not sure. So 'clean-up' will need to become a separate issue.This PR relies on changes in our NLDS token package PR nr.9 so to view and check to see if all styles look the same, do an NPM ci --legacy-peer-deps install and rebuild.
Note to self: the two unique classes
utrecht-paragraph--oip-title-text
andutrecht-paragraph--oip-summary
are not in design tokens becasue they should probably be defined by their parent component (like "readmore" component.