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

Add an option to use the Windows title bar #912

Merged
merged 1 commit into from
Oct 8, 2023
Merged

Conversation

mon-jai
Copy link

@mon-jai mon-jai commented Jul 27, 2023

Closes #285

Description

This pull request add an option to the preference dialog for enabling the Windows title bar.

Since restarting electron is required for changes to take effect, a dialog will be shown asking if user wanted to restart right away.

The two options, "custom" and "native" comes from the similar option in VS Code, so they should familiar enough for most users.

image

TODO:

  • Fix comments
  • Sort imports
  • Rebase changes to a single commit

Screenshots

Release notes

Notes:

@mon-jai mon-jai marked this pull request as draft July 27, 2023 20:07
@mon-jai
Copy link
Author

mon-jai commented Jul 27, 2023

Hi @shiftkey. Should we get his merged before #911, or vice versa?

@shiftkey
Copy link
Owner

@mon-jai given the progress of that PR, I think we can land this one after

Copy link
Owner

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Some early thoughts on the diff of changes, as I haven't had a chance to run this code up and explore the feature in it's current form...

app/src/lib/stores/app-store.ts Outdated Show resolved Hide resolved
app/src/lib/title-bar-config.ts Outdated Show resolved Hide resolved
app/src/lib/title-bar-config.ts Outdated Show resolved Hide resolved
app/src/lib/title-bar-config.ts Outdated Show resolved Hide resolved
app/src/ui/app.tsx Outdated Show resolved Hide resolved
app/src/ui/app.tsx Show resolved Hide resolved
app/styles/ui/_preferences.scss Outdated Show resolved Hide resolved
@mon-jai
Copy link
Author

mon-jai commented Jul 28, 2023

@mon-jai given the progress of that PR, I think we can land this one after

OK

app/src/ui/app.tsx Outdated Show resolved Hide resolved
app/src/ui/app.tsx Outdated Show resolved Hide resolved
app/src/ui/app.tsx Outdated Show resolved Hide resolved
app/src/ui/preferences/appearance.tsx Outdated Show resolved Hide resolved
@mon-jai
Copy link
Author

mon-jai commented Jul 30, 2023

@shiftkey I have rewritten app/src/lib/title-bar-config.ts to make it more in line with the API and caching design of app/src/lib/get-main-guid.ts.

Can you take a look and see if it is OK?

@mon-jai
Copy link
Author

mon-jai commented Jul 30, 2023

Screenshot of the updated UI:

_usr_lib_github-desktop_resources_app_index html

Edit: use unordered list instead

Edit 2: full stop fixed

@mon-jai mon-jai requested a review from shiftkey July 31, 2023 13:27
Copy link

@Tankbog Tankbog left a comment

Choose a reason for hiding this comment

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

  • [ ]

@mon-jai mon-jai marked this pull request as ready for review August 2, 2023 14:39
@mon-jai
Copy link
Author

mon-jai commented Aug 2, 2023

Double checked and squashed to a single commit. This should be ready for review.

Copy link
Owner

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

This is feeling really close - just some polish and suggestions to wrap this up before I slide this into the next release...

app/src/ui/preferences/appearance.tsx Outdated Show resolved Hide resolved
app/src/ui/preferences/confirm-restart.tsx Outdated Show resolved Hide resolved
app/src/ui/preferences/appearance.tsx Outdated Show resolved Hide resolved
app/src/lib/get-title-bar-config.ts Outdated Show resolved Hide resolved
app/src/main-process/main.ts Outdated Show resolved Hide resolved
app/src/ui/lib/title-bar-style.ts Show resolved Hide resolved
@shiftkey
Copy link
Owner

I've just rebased this branch on top of the default branch here as there were some conflicts with the last couple of releases, but this is otherwise feeling close. I'm happy to take it over and resolve the last couple of things if you don't have time currently @mon-jai!

@mon-jai
Copy link
Author

mon-jai commented Aug 27, 2023

Thanks @shiftkey. I have resolved all conversations but do not have time yet to check the changes on a Linux PC.

I may have time after one to two weeks. In the meanwhile, if you could verify that the PR works, feel free to merge it, just remember to keep credit to me with Co-authored-by in the commit message.

@mon-jai
Copy link
Author

mon-jai commented Sep 19, 2023

@shiftkey Sorry for the late reply.

I have made some final UI fixes, tested the changes, and squashed them into a single commit.

The pull request is now ready for merging, remember to keep credit to me with Co-authored-by in the commit message..

Testing screencast:

Screencast.from.09-19-2023.05.39.44.PM.webm

@mon-jai mon-jai requested a review from shiftkey September 19, 2023 10:35
@mon-jai
Copy link
Author

mon-jai commented Oct 4, 2023

Hi @shiftkey. Can you help reviewing the PR?

@shiftkey
Copy link
Owner

shiftkey commented Oct 4, 2023

Hi @shiftkey. Can you help reviewing the PR?

I haven't forgotten this, just haven't had the time to sit down and give this one last look over. I'm also due to cut a new upstream release so I'll make sure to do both at the same time (probably this weekend).

shiftkey added a commit that referenced this pull request Aug 12, 2024
shiftkey added a commit that referenced this pull request Aug 12, 2024
shiftkey added a commit that referenced this pull request Aug 12, 2024
shiftkey added a commit that referenced this pull request Aug 12, 2024
shiftkey added a commit that referenced this pull request Aug 12, 2024
shiftkey added a commit that referenced this pull request Aug 14, 2024
shiftkey added a commit that referenced this pull request Aug 15, 2024
shiftkey added a commit that referenced this pull request Aug 17, 2024
shiftkey added a commit that referenced this pull request Aug 17, 2024
shiftkey added a commit that referenced this pull request Sep 2, 2024
shiftkey added a commit that referenced this pull request Sep 2, 2024
shiftkey added a commit that referenced this pull request Oct 30, 2024
shiftkey added a commit that referenced this pull request Nov 8, 2024
shiftkey added a commit that referenced this pull request Nov 8, 2024
shiftkey added a commit that referenced this pull request Nov 8, 2024
shiftkey added a commit that referenced this pull request Nov 8, 2024
shiftkey added a commit that referenced this pull request Nov 8, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Nov 11, 2024
shiftkey added a commit that referenced this pull request Feb 5, 2025
shiftkey added a commit that referenced this pull request Feb 5, 2025
shiftkey added a commit that referenced this pull request Feb 5, 2025
shiftkey added a commit that referenced this pull request Feb 5, 2025
shiftkey added a commit that referenced this pull request Feb 5, 2025
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.

option to use Windows title bar on Linux
4 participants