Skip to content

Conversation

@krrish-sehgal
Copy link
Contributor

@krrish-sehgal krrish-sehgal commented Oct 16, 2025

fixes: #1837
It looks like this now:

Screenshot 2025-10-16 at 11 53 57 AM

How I computed the title

I used useMatches() (React Router v6) to get the active route matches.
From each match I try to call match.handle?.crumb() if present — this returns a string (or array of strings for multi-part crumbs in some routes).
I take the last non-null crumb from the matched route list and set that as document.title. If none found, fallback is "Alby Hub"


// Replace current history state to include title in state (some browsers show
// history entry title from state). We keep the existing state but add _title.
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems hacky to me, I would avoid changing the history state directly. Do you know which browsers need this?

Copy link
Contributor Author

@krrish-sehgal krrish-sehgal Oct 20, 2025

Choose a reason for hiding this comment

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

I was getting a bug that was getting fixed with this. But its not there anymore... seems like the issue was elsewhere.
I'll remove this , modern browsers don't need this, document.title is enough.

useRemoveSuccessfulChannelOrder();
useNotifyReceivedPayments();

// Update document.title and the history entry when the route changes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be moved to a separate hook? see the ones two lines above for examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes! nice idea!

// Attempt to derive a title from route handles (crumb) or matched route
// fallback to app name. Use a small typed helper instead of `any` to
// satisfy lint rules.
const getCrumbFromHandle = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can use crumbs. This is for breadcrumbs and does not accurately set the title for a specific page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rolznz , Would you like me to instead use .title from the handle field in routes.tsx? Should I add a title to every route which is just having the crumb field but missing the title field...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be OK, but one downside is it will not be dynamic as far as I can tell.

It might be better to wait until we update to React 19. CC @reneaaron

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense...

@rolznz
Copy link
Contributor

rolznz commented Oct 16, 2025

We haven't updated to React 19, but that seems a much cleaner way to set the page title:

https://react.dev/blog/2024/12/05/react-19#support-for-metadata-tags

@rolznz rolznz marked this pull request as draft October 16, 2025 08:59
@krrish-sehgal
Copy link
Contributor Author

Still working after the fix:
Screenshot 2025-10-20 at 1 49 08 PM

@krrish-sehgal krrish-sehgal marked this pull request as ready for review October 20, 2025 08:21
@rolznz
Copy link
Contributor

rolznz commented Oct 20, 2025

@krrish-sehgal I don't think this will work for app connections or app store apps because they have dynamic titles, right? I think we should wait for React 19 so we can do it properly.

@krrish-sehgal
Copy link
Contributor Author

okay , marking it as draft again, then, will wait for further instrcutions on this.
Thanks!

@krrish-sehgal krrish-sehgal marked this pull request as draft October 20, 2025 08:42
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.

Bug: Alby Hub does not include page title in history

2 participants