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 text in link fields #23

Merged
merged 26 commits into from
Sep 26, 2024
Merged

feat: add support for text in link fields #23

merged 26 commits into from
Sep 26, 2024

Conversation

levimykel
Copy link
Collaborator

Resolves: DT-2261

Description

Adds support for the display text property in link fields.

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

N/A

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 self-assigned this Aug 26, 2024
Copy link

github-actions bot commented Aug 26, 2024

size-limit report 📦

Path Size
dist/index.js 11.11 KB (+0.61% 🔺)
dist/index.cjs 14.41 KB (+0.59% 🔺)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.55%. Comparing base (2a7f0cb) to head (20c870c).
Report is 4 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   83.39%   83.55%   +0.15%     
==========================================
  Files          82       82              
  Lines        1060     1070      +10     
  Branches      234      241       +7     
==========================================
+ Hits          884      894      +10     
  Misses        120      120              
  Partials       56       56              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Great work here! I suggested quite a few changes, but they should be relatively quick to fix.

src/lib/buildContentRelationshipField.ts Show resolved Hide resolved
src/lib/buildContentRelationshipField.ts Outdated Show resolved Hide resolved
src/model/contentRelationship.ts Outdated Show resolved Hide resolved
src/model/contentRelationship.ts Outdated Show resolved Hide resolved
src/model/contentRelationship.ts Outdated Show resolved Hide resolved
src/model/linkToMedia.ts Outdated Show resolved Hide resolved
src/value/link.ts Outdated Show resolved Hide resolved
test/snapshots/lib-buildMockGroupFieldMap.test.ts.md Outdated Show resolved Hide resolved
test/value-contentRelationship.test.ts Outdated Show resolved Hide resolved
test/value-link.test.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
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.

LGTM! @dani-mp's small change for the unit:watch script would be nice to include if possible.

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! Just something weird happened with the whitespace in one file. This can be merged and published.

Copy link
Member

Choose a reason for hiding this comment

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

⚠️ #issue: Something weird happened in this file where all the whitespace was removed. Could we put it back? Sorry, I missed this in an earlier review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

weird indeed, i've put them back

…-in-links

# Conflicts:
#	CHANGELOG.md
#	package-lock.json
#	package.json
#	test/snapshots/lib-buildMockGroupFieldMap.test.ts.snap
#	test/snapshots/model-link.test.ts.snap
#	test/snapshots/model-linkToMedia.test.ts.snap
#	test/snapshots/value-image.test.ts.snap
#	test/snapshots/value-link.test.ts.md
#	test/snapshots/value-link.test.ts.snap
#	test/snapshots/value-linkToMedia.test.ts.md
#	test/snapshots/value-linkToMedia.test.ts.snap
#	test/snapshots/value-sharedSlice.test.ts.snap
#	test/snapshots/value-sharedSliceVariation.test.ts.snap
#	test/snapshots/value-slice.test.ts.snap
@dani-mp dani-mp merged commit 4625b23 into main Sep 26, 2024
2 checks passed
@dani-mp dani-mp deleted the lg/text-in-links branch September 26, 2024 13:02
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.

5 participants