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

[Fairground] Add a property to indicate the heading style. Replace the title property. #109

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

waisingyiu
Copy link
Contributor

What does this change?

We have to support at least three styles for a collection title for the new design of the homepage:

  1. hidden (for example, in highlights)
  2. in regular size
  3. in small size (for example, in small story carousels)

Previously, we tell the apps to hide the title by not populating the title property. However, we found that we need to show title elsewhere (for example, homepage customisation page) even if we don't show it on the front.

After discussion, we agree to create a new property to indicate in what style to display the title. In this schema, we should always provide the title. We have to add a new title property and deprecate the existing one in order not to break the existing version of the app.

@waisingyiu waisingyiu requested a review from a team as a code owner September 17, 2024 16:31
@waisingyiu waisingyiu force-pushed the fairground/add-heading-style-for-collections branch from d376bc6 to 77f4fe1 Compare September 17, 2024 16:34
* The title of this collection for display. It replaces
* the old "title" property.
*/
optional string display_title = 20;

Choose a reason for hiding this comment

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

Should title be marked as deprecated too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once the title is marked as deprecated, MAPI cannot access this property. But at this stage, we should still populate title in order not to break existing versions of the app.

Choose a reason for hiding this comment

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

Makes sense, thanks @waisingyiu

@gu-scala-library-release
Copy link
Contributor

@waisingyiu has published a preview version of this PR with release workflow run #71, based on commit 77f4fe1:

1.0.26-PREVIEW.fairgroundadd-heading-style-for-collections.2024-09-18T1127.77f4fe14

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the fairground/add-heading-style-for-collections branch, or use the GitHub CLI command:

gh workflow run release.yml --ref fairground/add-heading-style-for-collections

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

@waisingyiu waisingyiu merged commit 576ee7f into main Sep 18, 2024
9 checks passed
@waisingyiu waisingyiu deleted the fairground/add-heading-style-for-collections branch September 18, 2024 17:04
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