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

feat: add support for repeatable links in models #355

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

levimykel
Copy link
Contributor

@levimykel levimykel commented Oct 28, 2024

Resolves: Repeatable links project

Description

The first of 3 PRs to support an array of links as well as single links. It would allow devs to not manually wrap their link in a Group. It would also allow editors to have a better experience with a dedicated UI for multiple links.

This PR handles the model changes. The model adds a new repeat property to define if the link field is an array or not.

{
  "example_link": {
    "type": "Link",
    "config": {
      "label": "My Button Link",
      "placeholder": "My Button Link Placeholder",
      "repeat": false, // true to activate repetition
      "select": null,
      "allowTargetBlank": false,
      "allowText": true
    }
  }
}

Other PRS:

Checklist

  • If my changes require tests, I added them.
  • If my changes affect backward compatibility, it has been discussed.
  • If my changes require an update to the CONTRIBUTING.md guide, I updated it.

Preview

How to QA 1

Footnotes

  1. Please use these labels when submitting a review:
    ❓ #ask: Ask a question.
    💡 #idea: Suggest an idea.
    ⚠️ #issue: Strongly suggest a change.
    🎉 #nice: Share a compliment.

@levimykel levimykel changed the title Lg/repeatable links feat: add support for repeatable links in models Oct 30, 2024
Copy link
Member

@angeloashmore angeloashmore left a comment

Choose a reason for hiding this comment

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

Content relationship and link to media fields are included in this PR, but I thought we were only enhancing the link field. If we're allowing it on all link-link fields, let me know and I'll approve it as is.

Everything else looks good. 👍

@@ -19,5 +19,6 @@ export interface CustomTypeModelContentRelationshipField<
customtypes?: readonly CustomTypeIDs[]
tags?: readonly Tags[]
allowText?: boolean
repeat?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

❓ Are we allowing content relationships to be repeatable? I thought we weren't, but correct me if I'm wrong.

@@ -14,5 +14,6 @@ export interface CustomTypeModelLinkToMediaField {
placeholder?: string
select: typeof CustomTypeModelLinkSelectType.Media
allowText?: boolean
repeat?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

❓ Are we allowing link to media fields to be repeatable? I thought we weren't, but correct me if I'm wrong.

@angeloashmore angeloashmore marked this pull request as ready for review November 27, 2024 01:37
@dani-mp
Copy link
Contributor

dani-mp commented Nov 27, 2024

@angeloashmore changes applied

Copy link
Member

@angeloashmore angeloashmore 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! We can merge and publish when we are ready.

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.

3 participants