Skip to content
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 abstract page content skeleton #8

Merged
merged 15 commits into from
Aug 21, 2024
Merged

Conversation

MaHaWo
Copy link
Collaborator

@MaHaWo MaHaWo commented Aug 19, 2024

  • adds the visual element for the basic page described in Create basic content page #2 .
  • allows for passing in components to render them into the basic page
  • has no functionality at the moment for buttons or links.
  • created a dummy frontpage to have the AbstractContent component do something.

@MaHaWo
Copy link
Collaborator Author

MaHaWo commented Aug 20, 2024

One question remains for me: where to put stylings? It seems that the idea is to apply them directly to wherever you need them in your component, but that seems to be an oddly non-reusable paradigm, since component styling and component functions do not necessarily correlate 100%? And having a global style sheet in which to maintain everything seems like a recipe for desaster?

@MaHaWo MaHaWo requested a review from lkeegan August 20, 2024 11:31
@MaHaWo
Copy link
Collaborator Author

MaHaWo commented Aug 20, 2024

what tests would be necessary at this point? there is no functionality yet, so I didn't really write any tests either....

Copy link
Member

@lkeegan lkeegan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! I think that in general <script> is not the place for styling - I'd suggest putting them directly in class=... and if that would result in duplication I'd suggest making a re-usable component with this styling.

tailwind.config.ts Outdated Show resolved Hide resolved
Comment on lines 16 to 23
<Card class={cardStyle}>
<h5 class={headerStyle}>Was ist Mondey?</h5>
<p class={paragraphStyle}>
<i>Mondey</i> ist ein wissentschaftlich geprüftes Programm zure Dokumentation der Entwicklung von
Kindern bis 6 Jahren.
</p>
<Button class={buttonStyle}>Mehr Info <ArrowRightOutline class={arrowStyle} /></Button>
</Card>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to make custom Card component that can be reused here, with inline component styling, the header text as a prop, and a slot for the content?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was thinking about this. but given that this is a dummy page and Mrs. Pauen wanted to design the page herself, I didn´t go to these lengths. Or do you think we should use the cards therein/somewhere else? In that case I would suggest having a separate issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/lib/components/Frontpage.svelte Outdated Show resolved Hide resolved
@lkeegan
Copy link
Member

lkeegan commented Aug 20, 2024

what tests would be necessary at this point? there is no functionality yet, so I didn't really write any tests either....

Agree - we can look at adding some cypress ui tests at some point, but I wouldn't start doing this until we have pretty much solidified the design.

@MaHaWo MaHaWo merged commit 62f594a into main Aug 21, 2024
2 checks passed
@MaHaWo MaHaWo deleted the create-abstract-content-page branch August 21, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants