hypothesis / lms

LTI app for integrating with learning management systems
BSD 2-Clause "Simplified" License
46 stars 14 forks source link

Make sure dashboard page titles update when navigating #6463

Closed acelaya closed 2 months ago

acelaya commented 2 months ago

Closes #6458

Add a new usePageTitle hook that sets document.title, and use it in all dashboard page/section components to ensure the page title is in sync with what's being displayed.

This approach has the drawback that we'll have to remember to call usePageTitle with any future section we add, but I think that's going to be needed regardless the approach, as we'll have to define the title per section somewhere no matter what.

Testing steps

  1. Open the dashboard.
  2. Navigate to other sections and observe the page title gets updated.

Titles should be the same that the backend sets on initial server-side renders, but we could potentially get rid of the backend logic entirely.

robertknight commented 2 months ago

This approach has the drawback that we'll have to remember to call usePageTitle with any future section we add, but I think that's going to be needed regardless the approach, as we'll have to define the title per section somewhere no matter what.

I agree that some route-specific logic will be needed, since the title will often depend on data fetched by the route.

It would be useful if we could set things up in such a way as to make it obvious if a route fails to set the title. One simple approach might be to reset the title, in development, to an obvious placeholder when navigating to a new route.

acelaya commented 2 months ago

One simple approach might be to reset the title, in development, to an obvious placeholder when navigating to a new route.

Let me try this

acelaya commented 2 months ago

This approach has the drawback that we'll have to remember to call usePageTitle with any future section we add, but I think that's going to be needed regardless the approach, as we'll have to define the title per section somewhere no matter what.

I agree that some route-specific logic will be needed, since the title will often depend on data fetched by the route.

It would be useful if we could set things up in such a way as to make it obvious if a route fails to set the title. One simple approach might be to reset the title, in development, to an obvious placeholder when navigating to a new route.

This proved to be a bit tricky to do with the approach implemented here, since we have multiple side effects in different places.

I'm going to close this PR and instead use an approach where we define page titles in a central place. When a route is going to be rendered, we check the corresponding title from there, and render the placeholder otherwise.

This will even allow us to enforce the title via types, reducing the chances to forget it.