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 #1428 - Update Browser Tab Title When Notebook is Renamed #1496

Merged
merged 4 commits into from
May 28, 2024

Conversation

vrtnis
Copy link
Contributor

@vrtnis vrtnis commented May 27, 2024

πŸ“ Summary

This pull request addresses the issue where the browser tab title does not update when a notebook is renamed. Fixes #1428 .

πŸ” Description of Changes

  • Updated the handleFilenameChange function in the EditApp component to set document.title with the new filename.
  • Added a useEffect hook to update document.title whenever the filename changes, ensuring the browser tab title stays in sync with the notebook name.
  • Added a default value ("Untitled Notebook") for the document title in case the filename is null.
marimotab1-2024-05-23_12.53.43.mp4

πŸ“‹ Checklist

  • [βœ“ ] I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made.
  • [βœ“ ] I have run the code and verified that it works as expected.

πŸ“œ Reviewers

@akshayka #1428

Copy link

vercel bot commented May 27, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
marimo-docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 28, 2024 2:56am
marimo-storybook βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback May 28, 2024 2:56am

Copy link

github-actions bot commented May 27, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ βœ…

@vrtnis
Copy link
Contributor Author

vrtnis commented May 27, 2024

I have read the CLA Document and I hereby sign the CLA

Copy link
Contributor

@akshayka akshayka left a comment

Choose a reason for hiding this comment

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

Thank you for this PR!

There's one more thing to handle. Notebooks can already override the browser tab title with the app title configuration. If an app title is already set, then that should take precedence.

App titles were added in this PR: #1264

@vrtnis
Copy link
Contributor Author

vrtnis commented May 28, 2024

Got it! thanks for the feedback, and for pointing out PR #1264, which introduced the app title feature!

I 've updated the implementation in 3dcb623 to ensure that the app title (appConfig.app_title) takes precedence over the notebook filename when setting the browser tab title. The document title is now set according to the following priority:

  • appConfig.app_title (if set)
  • filename (if appConfig.app_title is not set)
  • Default to "Untitled Notebook" (if neither is set)

BTW this is handled both during the renaming process and whenever the app title or filename changes.

return sendRename(name)
  .then(() => {
    setFilename(name);
    // Set document title: app_title takes precedence, then filename, then default
    document.title = appConfig.app_title || name || "Untitled Notebook"; 
    return name;
  })
  .catch((error) => {
    openAlert(error.message);
    return null;
  });

// Update document title whenever filename or app_title changes 
useEffect(() => {
  // Set document title: app_title takes precedence, then filename, then default
  document.title = appConfig.app_title || filename || "Untitled Notebook"; 
}, [appConfig.app_title, filename]);
marimopr1-2024-05-27_18.37.48.mp4

Copy link
Contributor

@mscolnick mscolnick left a comment

Choose a reason for hiding this comment

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

looks great- thanks for the fix!

@mscolnick mscolnick merged commit 692b777 into marimo-team:main May 28, 2024
26 checks passed
Copy link

πŸš€ Development release published. You may be able to view the changes at https://marimo.app?v=0.6.11-dev4

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.

Renaming notebook doesn't change title of tab
3 participants