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

fix app title logic #371

Merged
merged 11 commits into from
Nov 25, 2024
Merged

fix app title logic #371

merged 11 commits into from
Nov 25, 2024

Conversation

illfixit
Copy link
Collaborator

@illfixit illfixit commented Nov 12, 2024

What issues does this PR close?
closes #237 Website title does not update in some scenarios

Unauthenticated
image

Onboarding (both organization and user)
image

Pending
image

Rejected
image

404 Page not found (page + title)
image

Logged In
image

Checklist

Preview Give feedback

@illfixit illfixit linked an issue Nov 12, 2024 that may be closed by this pull request
@illfixit illfixit enabled auto-merge (squash) November 12, 2024 15:15
Copy link
Collaborator

@kamilczaja kamilczaja left a comment

Choose a reason for hiding this comment

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

Implementation-wise, that looks good.

I'm not sure though if this is okay from a business perspective, namely "MDS Portal" being removed from the title. This PR does more than is laid out in the connected issue.

Someone should clarify if this is okay.

@jkbquabeck @AbdullahMuk

richardtreier pushed a commit that referenced this pull request Nov 18, 2024
@illfixit
Copy link
Collaborator Author

illfixit commented Nov 18, 2024

Implementation-wise, that looks good.

I'm not sure though if this is okay from a business perspective, namely "MDS Portal" being removed from the title. This PR does more than is laid out in the connected issue.

Someone should clarify if this is okay.

@jkbquabeck @AbdullahMuk

MDS part is just a tiny bug after extracting it into separate PR, we can fix it very quick.
I agree that it's a little more than in bug and the issue itself came internally iirc (from JK), not from MDS.

@illfixit
Copy link
Collaborator Author

@kamilczaja MDS is back in Titles

Copy link
Collaborator

@kamilczaja kamilczaja left a comment

Choose a reason for hiding this comment

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

LGTM

@illfixit illfixit merged commit e332338 into main Nov 25, 2024
11 of 12 checks passed
@illfixit illfixit deleted the 237-website-title-does-not-update branch November 25, 2024 12:57
kamilczaja pushed a commit that referenced this pull request Dec 3, 2024
(cherry picked from commit e332338)
kamilczaja pushed a commit that referenced this pull request Dec 9, 2024
(cherry picked from commit e332338)
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.

Website title does not update in some scenarios
2 participants