Kiyozz / dofus-db-treasure-hunt-overlay

Provides an overlay for Dofus DB Treasure Hunt. Useful for user with one monitor wanted to use DofusDB easily.
8 stars 1 forks source link

added option to change language between English and French #13

Closed Skijearz closed 2 years ago

Skijearz commented 2 years ago

Hey, in order to use this tool i needed a English version of it. This could simply be done by changing the URL which the window loads on startup but this would only be a temporary "fix". I added a button-dropdown menu to the top-layout and added English and French as options.

The method to handle the clicks,on those options , could be optimized because right now in my solution i created 2 new methods in the ipcRendererinstead of 1 wich would take arguments, unfortunately i never used electron to build apps like this therefore i actually dont know how to modify the ipcRendererto take arguments.

Kiyozz commented 2 years ago

Thanks you Skijearz. Let me review your changes.

Please makes dropdown dynamic: use a constant to register all its options. You can register the current language in a let and use it to load the url.

Revert the default loaded url. You can use electron-store to save the current used language in a json. Use dmo as store filename.

If you do not know how to do all of that, I can take your branch to improve it!

Thanks again.

Skijearz commented 2 years ago

Added a new Commit which uses Electron-store to persist the chosen language between restarts and reverted the default back to French. But could you explain what you mean with :

Please makes dropdown dynamic: use a constant to register all its options. You can register the current language in a let and use it to load the url.

im not 100% sure what you mean with that, but would happily do that changes ! :D

Skijearz commented 2 years ago

Additionally the ipcRenderer now utilizes a language parameter so there is no need for 2 separate changeLanguage-functions anymore. Useful if more languages will be added.

Skijearz commented 2 years ago

i just discovered a strange thing and i actually dont know why that happens. the 2 elements used for changing the language are only clickable as long as there is the input from the coordinates behind it. image

but as soon as my cursor is between the inputs or you scale the window larger they stop working(you can see there is no highlighting anymore and i cant click on them) image

do you have any idea why that might be the case ?

Kiyozz commented 2 years ago

i just discovered a strange thing and i actually dont know why that happens. the 2 elements used for changing the language are only clickable as long as there is the input from the coordinates behind it. image

but as soon as my cursor is between the inputs or you scale the window larger they stop working(you can see there is no highlighting anymore and i cant click on them) image

do you have any idea why that might be the case ?

I pulled your PR, I do not have this problem.

Skijearz commented 2 years ago

Thats a weird thing, i cant seem to fix it on my end then ~ would you mind building my pr for me to test your build then ?

Kiyozz commented 2 years ago

Which system are you using?

I'm on macOS (when I did the test)

Skijearz commented 2 years ago

Win11

Skijearz commented 2 years ago

Okay the problem was the missing "app-region:no-drag" on the dropdown menu. Now its working for me too :D

Skijearz commented 2 years ago

Now im quite happy with all the changes i made ! :D only thing i can think of would be to hide the re-loading of the website on language change but actually i have no idea how we could do that, can only think of win?.hide() but that's not a great solution imo.

Kiyozz commented 2 years ago

For the scope of this app, hiding the reloading does not worth it. Let me re-format the code and merge your PR.

Kiyozz commented 2 years ago

In the futur, please use pnpm in this project

Skijearz commented 2 years ago

In the futur, please use pnpm in this project

Yea sure, but how did you know i didn't use pnpm ? :D

Kiyozz commented 2 years ago

package-lock :)

Kiyozz commented 2 years ago

package-lock :)

Please merge the pr in your fork so I can merge this