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

adds possibility to add link to card #185

Merged
merged 8 commits into from Nov 22, 2022

Conversation

MariaBee3
Copy link
Collaborator

This pull request adds the possibility in the CMS to add a link to a card in the cardblock.

@MariaBee3 MariaBee3 linked an issue Nov 17, 2022 that may be closed by this pull request
3 tasks
@MariaBee3 MariaBee3 changed the title adds possibility to add link to card draft: adds possibility to add link to card Nov 17, 2022
@MariaBee3 MariaBee3 requested a review from antw November 18, 2022 08:57
@MariaBee3
Copy link
Collaborator Author

@antw would you be willing to review this issue? I've been doubting about wrapping a card into a link or making a stretchable link in the card. I couldn't find a conclusive answer online, so for now I've made a conditional wrapper that links the card if a link has been added in the CMS. But I would be very interested in hearing you opinion on this. And of course on any other issue you come across.

@MariaBee3 MariaBee3 changed the title draft: adds possibility to add link to card adds possibility to add link to card Nov 18, 2022
@antw
Copy link
Contributor

antw commented Nov 18, 2022

I've been doubting about wrapping a card into a link or making a stretchable link in the card.

Personally, I'd use something like a stretched link. Wrap whatever part of the card you want to be clickable in className="relative" then apply the stretched link class to the link.

/* styles/globals.css */

@layer base {
  .stretched-link::after {
    content: "";
    @apply z-10 absolute inset-0 bg-transparent pointer-events-auto;
  }
}
// components/StretchedLink.tsx

function StretchedLink({ content, className, ...linkProps }: React.ComponentProps<'a'>) {
  return (
    // Use "a" or Link as appropriate.
    <a {...linkProps} className={`${className || '' } stretched-link`}>
      {content}
    </a>
  );
}
// Inside the component

function CardTitle({ condition, children, ...rest }) {
  // Pseudo code
  if (condition) {
    return <StretchedLink href="..." className="mb-3 block">...</StretchedLink>
  }

  return <strong className="mb-3 block>...</strong>;
}

function Card(...)
  return (
    {/* stuff */}
    <div className="flex-col flex m-4 flex-1 max-h:1/2 overflow-hidden">
      <CardTitle ...>{cardItem.title}</CardTitle>
      <div>
        <RawHtml html={cardItem.text} />
      </div>
    </div>
    {/* more stuff */
  );
}

@MariaBee3
Copy link
Collaborator Author

@antw Thanks for the feedback. Good idea to make it a separate component. I've implemented your suggestion. Could you please review it?
There is still a problem in this branch with the migration files, causing a failed check, we are solving this currently.

antw
antw previously approved these changes Nov 22, 2022
@MariaBee3
Copy link
Collaborator Author

@broekhuisg wil jij misschien deze nog goedkeuren? Anthony heeft alles al gereviewed en goedgekeurd maar omdat ik de laatste migration files nog heb gepushed moet er weer iemand het goedkeuren.

@MariaBee3
Copy link
Collaborator Author

@antw Sorry, would you mind approving this issue again? We had to change some things in the migration files before it would pass all tests. I've pushed this and now it needs to be approved again. Nothing changed in the code you approved earlier.

@MariaBee3 MariaBee3 merged commit 08ab9ca into main Nov 22, 2022
@mattijsstam mattijsstam deleted the 137-add-links-to-cards-in-cardblock branch November 29, 2022 08:21
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.

Add links to cards in cardblock
2 participants