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.

Sign up for GitHub

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

Jump to bottom

Desktop: resolve #3695: Show notebook and note title in the title bar #4390

Merged
merged 9 commits into from Apr 6, 2021
Merged

Desktop: resolve #3695: Show notebook and note title in the title bar #4390

merged 9 commits into from Apr 6, 2021

Conversation

asrient
Copy link
Contributor

@asrient asrient commented Jan 19, 2021

Titlebar of the desktop electron app now shows the name of notebook and note title you are currently viewing.
Screenshot 2021-01-19 at 5 25 21 PM

The title convention used is: Note (Notebook) - Joplin

@asrient asrient changed the title Desktop: Resolves #3695: Show notebook and note title in the title bar Desktop: resolve #3695: Show notebook and note title in the title bar Jan 19, 2021
laurent22
laurent22 reviewed Jan 19, 2021
View reviewed changes
Copy link
Owner

@laurent22 laurent22 left a comment

Choose a reason for hiding this comment

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

Thanks for the update. There are a few issues which I've described in the comments below. Also please add unit tests for that new code.

packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
laurent22
laurent22 reviewed Jan 20, 2021
View reviewed changes
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
laurent22
laurent22 requested changes Jan 27, 2021
View reviewed changes
packages/app-desktop/gui/getWindowTitle.ts Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.test.ts Outdated Show resolved Hide resolved
@laurent22
Copy link
Owner

@asrient, are you still interested in completing this pull request?

@asrient
Copy link
Contributor Author

asrient commented Feb 6, 2021

Yes. I will complete asap.

@tessus tessus added the desktop All desktop platforms label Feb 20, 2021
laurent22
laurent22 reviewed Feb 23, 2021
View reviewed changes
.eslintignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.test.ts Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.ts Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.ts Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.test.ts Outdated Show resolved Hide resolved
packages/app-desktop/gui/Navigator.jsx Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.ts Outdated Show resolved Hide resolved
packages/app-desktop/gui/getWindowTitle.ts Outdated Show resolved Hide resolved
@roman-r-m roman-r-m mentioned this pull request Mar 12, 2021
Closed
@asrient asrient mentioned this pull request Mar 12, 2021
Closed
@asrient asrient requested a review from laurent22 March 21, 2021 04:38
@laurent22
Copy link
Owner

It's good now, but you've somehow bypassed the linter on the last commit so CI is failing.

@laurent22
Copy link
Owner

All good now, many thanks for the pull request @asrient!

@laurent22 laurent22 merged commit c79c9c4 into laurent22:dev Apr 6, 2021
laurent22 added a commit that referenced this pull request May 4, 2021
@laurent22
Revert "Desktop: Resolve #3695: Show notebook and note title in the t…
a0ead2c 
…itle bar (#4390)"

This reverts commit c79c9c4.

Reason: Still need to discuss how it should be implemented. Ref:

https://discourse.joplinapp.org/t/testing-v1-8-2-title-bar-path-format-minor/16937/10
@laurent22
Copy link
Owner

Unfortunately I had to revert this change as I realise we didn't discuss much what is the exact reason for this change, and how it should be implemented. Once it's clarified we can see about adding it back. Ref: https://discourse.joplinapp.org/t/testing-v1-8-2-title-bar-path-format-minor/16937/10

Revert commit: a0ead2c

@KoolKeith
Copy link

Hello.
I need this, too. Is it already live and working, in the current release? Where is the setting in options?
Thank you very much for Jopin !

@bkidwell
Copy link

Naming the notebook somehow in the window title is a critical usability feature to enable a user to have separate "Work" and "Personal" databases, in combination with the "--profile" command line switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurent22 laurent22 Awaiting requested review from laurent22

Assignees
No one assigned
Labels
desktop All desktop platforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@asrient @laurent22 @KoolKeith @bkidwell @tessus