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

Default universal section title to vertical's displayName. #493

Merged
merged 1 commit into from
Nov 24, 2020

Conversation

tmeyer2115
Copy link
Collaborator

Recently, the JAMBO_INJECTED_DATA was updated so that the displayName of
each vertical in an experience is provided. We should default to this name
when constructing the vertical's universal section template. As before, if
a label or secionTitleName has been specified in page configuration, we
will favor those instead.

J=SLAP-887
TEST=manual

Tested the following:

  • Set a label for a vertical, verified that was used as the section title.
  • Set a sectionTitleName for a vertical, verified that was used as the
    section title.
  • When no label or sectionTitleName was present, saw the displayName from
    the injected data used as the section title.
  • If no diplsayName was present for the vertical, saw it's verticalKey used
    as the section title.

Recently, the `JAMBO_INJECTED_DATA` was updated so that the `displayName` of
each vertical in an experience is provided. We should default to this name
when constructing the vertical's universal section template. As before, if
a `label` or `secionTitleName` has been specified in page configuration, we
will favor those instead.

J=SLAP-887
TEST=manual

Tested the following:

- Set a `label` for a vertical, verified that was used as the section title.
- Set a `sectionTitleName` for a vertical, verified that was used as the
  section title.
- When no `label` or `sectionTitleName` was present, saw the `displayName` from
  the injected data used as the section title.
- If no `diplsayName` was present for the vertical, saw it's `verticalKey` used
  as the section title.
Copy link
Contributor

@creotutar creotutar left a comment

Choose a reason for hiding this comment

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

🤠 🤠 🤠 LGTM

Copy link
Contributor

@nanhu95 nanhu95 left a comment

Choose a reason for hiding this comment

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

LGTM!

@tmeyer2115 tmeyer2115 merged commit 0c85480 into release/v1.17 Nov 24, 2020
@tmeyer2115 tmeyer2115 deleted the dev/defaulted-section-title branch November 24, 2020 15:28
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