Page MenuHomePhabricator

[dagit] Page titles for all routes
ClosedPublic

Authored by dish on Mon, Oct 5, 3:29 PM.

Details

Summary

Resolves #2998

Set document titles for all Dagit routes using a React hook.

Test Plan

Lint, ts, jest.

Run dagit, click through parts of the app. Verify that the document title changes, including on back/forward navigation.

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dish requested review of this revision.Mon, Oct 5, 3:33 PM

This looks great! In the future it might be a good idea to merge this with RunStatusToPageAttributes, which is responsible for turning the Dagit favicon green / red / gray based on the status of the current run.

Edit: actually, it looks like RunStatusToPageAttributes is also setting the page title for the run details page to (pipelineName runId [status]), maybe we can pull that responsibility out or give useDocumentTitle an optional favicon param. I think the only caveat is that the RunStatusToPageAttributes hack is currently within the auto-reloading graphql query so it reflects the up-to-date state of the run, we might have to move the new hook down a layer.

(Also please disregard how terrible RunStatusToPageAttributes is, I don't think we'd updated the project to use hooks yet back when I added it! ๐Ÿ™)

This revision is now accepted and ready to land.Mon, Oct 5, 7:30 PM

@bengotow No problem! We'll need to think through how to make favicons a bit more customizable anyway: https://github.com/dagster-io/dagster/issues/2994. I'm going to leave that page alone for now, and we can discuss further. :)

This revision was landed with ongoing or failed builds.Mon, Oct 5, 8:23 PM
This revision was automatically updated to reflect the committed changes.