-
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
Add EventPage
+ EventHeader
#353
Add EventPage
+ EventHeader
#353
Conversation
909b05c
to
834c667
Compare
092a3fb
to
5170a07
Compare
This should be reverted when the pull requests in dpl-design-system are merged. danskernesdigitalebibliotek/dpl-design-system#353
This should be reverted when the pull requests in dpl-design-system are merged. danskernesdigitalebibliotek/dpl-design-system#353
This should be reverted when the pull requests in dpl-design-system are merged. danskernesdigitalebibliotek/dpl-design-system#353
5170a07
to
aed6a2b
Compare
This should be reverted when the pull requests in dpl-design-system are merged. danskernesdigitalebibliotek/dpl-design-system#353
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.
👍 Good stuff here.
I appreciate you doing an effort to use the right HTML elements. I think we can do even better.
c6f529e
to
305f262
Compare
This should be reverted when the pull requests in dpl-design-system are merged. danskernesdigitalebibliotek/dpl-design-system#353
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 like the new changes.
The only thing I that needs to be addressed now is the usage of <figure>
.
.event-header__content { | ||
display: grid; | ||
grid-template-columns: $s-3xl 1fr $s-3xl; | ||
grid-template-areas: | ||
". tag ." | ||
". date ." | ||
". title ." | ||
"btn btn btn"; |
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.
As I read this you are creating padding for some areas using columns. That's creative.
Implemented a new SCSS mixin named 'container' to handle different container sizes: small, medium, and large. This mixin allows for dynamic styling based on the specified size parameter. Styles for 'small' and 'large' sizes are structured for future customization, while the 'medium' size is currently set with a max-width of 1440px and centered margin.
Introduced markup and CSS for `EventHeader`, now included in `EventPage`. Additionally, made small modifications to the `tag` component to allow the addition of extra CSS classes. In typography, I introduced a placeholder for `.text-header-h1` to facilitate future extension of this class
Enabling user experimentation with various images.
CSS handles uppercase transformation
-Removed the `className` property from the `Button` component due to redundancy. - Planned additional refactoring to extend the "button" classes within `event-header__button`
Extracted into a separate component to facilitate easy integration in diverse application areas.
…as` for content positioning. Removed unnecessary wrapper div in `.event-header__content` for better semantic structure
305f262
to
2c7aa0a
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.
👍 LGTM
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.
👍 LGTM
Link to issue
https://reload.atlassian.net/browse/DDFFORM-14
Description
This pull request introduces two new components:
EventPage
andEventHeader
. These components are used for the event-related pages of our application.Screenshot of the result