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

CCDM: allow @PageTitle to be in app-shell and views #7266

Merged
merged 1 commit into from
Jan 3, 2020

Conversation

manolo
Copy link
Member

@manolo manolo commented Jan 2, 2020

This change is Reviewable

@manolo manolo added the hilla Issues related to Hilla label Jan 2, 2020
@manolo manolo self-assigned this Jan 2, 2020
@project-bot project-bot bot added this to External Reviews in OLD Vaadin Flow ongoing work (Vaadin 10+) Jan 2, 2020
Copy link
Contributor

@haijian-vaadin haijian-vaadin left a comment

Choose a reason for hiding this comment

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

What will happen if there is a @PageTitle annotation on AppShell? Will it override other @PageTitles or others will override the one on AppShell?

Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained (waiting on @haijian-vaadin, @ptdatkhtn, and @vlukashov)

Copy link

@vlukashov vlukashov left a comment

Choose a reason for hiding this comment

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

FYI:
When placed on the app shell class this annotation controls the title returned in the index.html response.
When placed on a route this annotation controls the title set dynamically when the route loads.

In practice, if there are different @PageRoute annotations on the app shell and on the currently rendered route, the browser would initially display the title from the app shell but then quickly switch to the title from the route. In most cases that should be barely noticeable for end users.

Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained (waiting on @ptdatkhtn and @vlukashov)

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from External Reviews to Reviewer approved Jan 3, 2020
Copy link

@vlukashov vlukashov left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained (waiting on @ptdatkhtn and @vlukashov)

@manolo manolo merged commit edcf0f7 into ccdm Jan 3, 2020
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Done - pending release Jan 3, 2020
@manolo manolo deleted the mcm/ccdm/fix-page-title branch January 3, 2020 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hilla Issues related to Hilla +0.0.1
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

None yet

4 participants