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 fields for tag list page #139

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lindseydew
Copy link
Contributor

What does this change?

This adds a HEADER object that will be used in the List endpoint in order to present the new tag page

new-tag-page

How to test

How can we measure success?

Have we considered potential risks?

Images

Accessibility

@lindseydew lindseydew requested a review from a team as a code owner December 12, 2024 16:03
@gu-scala-library-release
Copy link
Contributor

@lindseydew has published a preview version of this PR with release workflow run #125, based on commit d540757:

1.0.42-PREVIEW.ldadd-fields-for-tag-list-page.2024-12-12T1618.d540757a

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the ld/add-fields-for-tag-list-page branch, or use the GitHub CLI command:

gh workflow run release.yml --ref ld/add-fields-for-tag-list-page

Want to make a full release after this PR is merged?

Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command:

gh workflow run release.yml

optional MyGuardianFollow my_guardian_follow = 2;
optional string description = 3;
HeaderType alignment = 4;
optional Branding branding = 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

The message List has a property branding too. Are there scenarios they would be different? Can we use the branding there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, there is a bit of a duplication here.

The idea behind it was to make the Header message atomic and with all the fields in order to be able to render it in different contexts.

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