Add Contact Us page, refactoring #1899
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
| return ( | ||
| <> | ||
| <header className="mx-auto w-full max-w-4xl pb-8 @2xl:pb-12 @4xl:pb-16"> | ||
| {breadcrumbs && <Breadcrumbs breadcrumbs={breadcrumbs} />} | ||
|
|
||
| <h1 className="mb-4 mt-8 font-heading text-4xl font-medium leading-none @xl:text-5xl @4xl:text-6xl"> | ||
| {title} | ||
| </h1> | ||
| </header> | ||
| <h1 className="mb-4 mt-8 font-heading text-4xl font-medium leading-none @xl:text-5xl @4xl:text-6xl"> | ||
| {title} | ||
| </h1> | ||
| </header> | ||
|
|
||
| <div | ||
| className="prose mx-auto w-full max-w-4xl space-y-4 [&_h2]:font-heading [&_h2]:text-3xl [&_h2]:font-normal [&_h2]:leading-none [&_h2]:@xl:text-4xl [&_img]:mx-auto [&_img]:max-h-[600px] [&_img]:w-fit [&_img]:rounded-2xl [&_img]:object-cover" | ||
| dangerouslySetInnerHTML={{ __html: content }} | ||
| /> | ||
| </> | ||
| ); | ||
| }} | ||
| </Stream> | ||
| </div> | ||
| <div | ||
| className="prose mx-auto w-full max-w-4xl space-y-4 [&_h2]:font-heading [&_h2]:text-3xl [&_h2]:font-normal [&_h2]:leading-none [&_h2]:@xl:text-4xl [&_img]:mx-auto [&_img]:max-h-[600px] [&_img]:w-fit [&_img]:rounded-2xl [&_img]:object-cover" | ||
| dangerouslySetInnerHTML={{ __html: content }} | ||
| /> | ||
| {children} | ||
| </> | ||
| ); |
There was a problem hiding this comment.
All these styles should be handled in VIBES so that we can swap out different VIBES and things won't look visually off.
There was a problem hiding this comment.
@migueloller Can you clarify this a bit? We decided that the base web page component shouldn't be in VIBES, so what's the best way to ensure the styles in Catalyst are sync'd with the vibes styles?
There was a problem hiding this comment.
@migueloller For some context, I grabbed that styling directly from the blog post content VIBES component here: https://github.com/makeswift/vibes/blob/85e9a51900472a0b5796731b9437781ef264b0b4/apps/web/vibes/soul/sections/blog-post-content/index.tsx#L92
There was a problem hiding this comment.
I was under the impression that webpage is not a section that will live in VIBES, so I asked Jordan to just style this page as close to the vibe as possible (maybe using SectionLayout).
However, I like the concept Jordan introduced of DynamicFormSection that has a title and subtitle and the form. This is something that should live in VIBES I think, since there are many use cases were we just need a title and form.
What do you think @migueloller ?
There was a problem hiding this comment.
@jorgemoya I did actually add DynamicFormSection into VIBES: makeswift/vibes#468
However, I ended up not using that section for Contact Page specifically, because of styling issues. For the contact page, we don't need the whole section, just the form, so it made more sense to just go with DynamicForm directly.
There was a problem hiding this comment.
Sorry for chiming in late here. Basically, the idea is that we want to be able to "hot-swap" VIBES when installing Catalyst in the future.
For this to work, we'll need to make sure Catalyst is using a curated interface that all VIBES will follow.
If we write custom CSS, while that might look good for the Soul VIBE it might not for another VIBE.
So, we'd need to identify what is that "shared" aspect that makes sense to move to VIBES. Perhaps it's a container/layout component, or something else. But by having it be part of the standard interface it would make it so VIBES as "hot-swappable".
Does that make sense?
There was a problem hiding this comment.
In this case, which is contrary to what I suggested Jordan so it's my bad, we need to build a VIBES component that has a title and subtitle and renders children.
jordanarldt
force-pushed
the
ja/add-contact-page-canary
branch
from
1a4f316 to
32283b4
Compare
jordanarldt
force-pushed
the
ja/add-contact-page-canary
branch
from
32283b4 to
f5f8111
Compare
core/app/[locale]/(default)/webpages/contact/[id]/_actions/submit-contact-form.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
This looks good to me but if @migueloller feels this WebPageContent component needs to be in VIBES, lets work to add it as well.
@jorgemoya For sure, we actually had a prior discussion about why it shouldn't be a vibes component, so we should be good to move forward 👍 |
What/Why?
SignUpFormtoDynamicFormSectionTesting
Screen.Recording.2025-01-10.at.6.17.33.PM.mov