-
Notifications
You must be signed in to change notification settings - Fork 30
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
Use local copy of article format types in DCR #12461
Conversation
@guardian/libs
Size Change: -23.3 kB (-2.54%) Total Size: 897 kB
ℹ️ View Unchanged
|
d7db13a
to
e709576
Compare
e709576
to
236a2ef
Compare
d9eb7e4
to
ddf047f
Compare
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
66101ce
to
9790f09
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.
Looks good! Couple of questions about schema changes.
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.
Why has this schema file changed?
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 not entirely sure to be honest! After I finished updating the imports it said the schema no longer matched and I should run make gen-schema
to update. It looks like it has been reordered rather than anything being added or removed though. Could it be because of the import order changing? I've tried reverting the changes to match what's currently in main
, but it failed schema validation again.
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.
Why has this schema file changed?
## What are you changing? - Remove the EditorialButton and EditorialLinkButton component, related tests and stories ## Why? - EditorialButton has only ever been used in [one place in DCR](https://github.com/guardian/dotcom-rendering/blob/4f75a0dbaf5a5dd756b1e8a1bb0aafa542e398a3/dotcom-rendering/src/components/Toast.tsx#L5). - We are moving format-related types into DCR (guardian/dotcom-rendering#12461). Removing this component removes one usage of format outside of DCR
## What are you changing? - Remove the QuoteIcon component, related tests and stories, and the HeadlineSize type - Extra bit: Fix the Logo README title, which was previously mistakenly "QuoteIcon" ## Why? - QuoteIcon and HeadlineSize are not used anywhere. DCR uses [its own implementation](https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/components/QuoteIcon.tsx) - We are moving format-related types into DCR (guardian/dotcom-rendering#12461). Removing this component removes one usage of format outside of DCR
Seen on PROD (created by @jamesmockett and merged by @SiAdcock 8 minutes and 30 seconds ago) Please check your changes! |
What does this change?
@guardian/libs
:ArticleDesign
ArticleDisplay
ArticleSpecial
ArticleTheme
ArticleFormat
Pillar
Why?
A small number of Source Development Kitchen components expose a
format
prop which is typed asArticleFormat
. Due to the need for these types to be used by Source and DCR they currently live in@guardian/libs
, although DCR is the main consumer. We want to decouple this dependency by refactoring the Source components to replace theformat
prop with a more generic theming mechanism, not tied to article formats, and move the types from@guardian/libs
to the DCR codebase where they can be more easily updated.This is part of a larger piece of work to remove
PrintShop
fromArticleDesign
.