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 new design user menu #1910

Merged
merged 3 commits into from
Oct 31, 2022
Merged

Conversation

dusansimic
Copy link
Collaborator

@dusansimic dusansimic commented Sep 10, 2022

This patch fixes the query selector for user menu on new design.

Checklist:

  • Fix user menu query
  • Check for new design
  • Clean up selectors

Closes: #1909
Closes: #1859
Closes: #1918
Closes: #1923

@dusansimic
Copy link
Collaborator Author

So here's a progress update. Right now I'm wrestling with typescript config because some packages that I'm bumping in this pr (for example element-ready) have switched to ES module system which is now causing trouble because for some reason, electron is importing our source code as a commonjs module. I'm still trying to figure out how to manage this but if anyone has any info or some examples, help is very welcome.

@dusansimic
Copy link
Collaborator Author

One solution that seems to work for some people is splitting the build process for main electron code, preloaded electron code and renderer code. All of those parts are build with vite and tsc (repo). It's worth looking into since it would clean up the code structure a bit and it might be a bit easier to maintain. That would however require adding a few extra steps to the build process.

@dusansimic
Copy link
Collaborator Author

A short term solution could be an user set preference for the new desing (if users have the new design, they can just chech the option in advanced settings in menu bar). It would require user intervention but it would fix the issue for now without refactoring the code to vite + tsc.

Copy link
Collaborator

@lefterisgar lefterisgar left a comment

Choose a reason for hiding this comment

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

It seems to fix said issues. Looks good to me!

@dusansimic
Copy link
Collaborator Author

@lefterisgar if you still have access to the old sidebar design, please test weather checking for new design works. It should work okay with openning and closing the preferences and it should remove the hide-preferences-window class once it's done.

@dusansimic dusansimic added the hacktoberfest-accepted PR is accepted for hacktoberfest label Oct 11, 2022
@lefterisgar
Copy link
Collaborator

@lefterisgar if you still have access to the old sidebar design, please test weather checking for new design works. It should work okay with openning and closing the preferences and it should remove the hide-preferences-window class once it's done.

Unfortunately I don't have access to it anymore. I think it has rolled out to most users out there.

@lefterisgar
Copy link
Collaborator

Is this ready to merge?

@dusansimic
Copy link
Collaborator Author

Is this ready to merge?

Essentially yes, I'd just like to complete the last task from the list in this pr. I'll see if I can do it today.

@dusansimic dusansimic merged commit 89c3a0b into sindresorhus:main Oct 31, 2022
@dusansimic dusansimic deleted the fix-new-design-menu branch October 31, 2022 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted PR is accepted for hacktoberfest
Projects
None yet
2 participants