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

bugfix: ribbon menu translations hot update #5633

Closed

Conversation

shun2wang
Copy link
Contributor

Fix: https://github.com/jasp-stats/INTERNAL-jasp/issues/2372

So we just remove/renew the menu ribbon button to "reload" it to apply the translations.

@boutinb
Copy link
Contributor

boutinb commented Aug 28, 2024

Really nice!
Just remove the 6 Modules/pkgdepends.lock of your PR.

@shun2wang
Copy link
Contributor Author

shun2wang commented Aug 28, 2024

Just remove the 6 Modules/pkgdepends.lock of your PR.

Well, it should be updated all because it block build on Linux.

Or someone can update all packages in pkgdepends.lock.

@RensDofferhoff
Copy link
Contributor

@shun2wang current pkgdepends does not build for you?

@shun2wang
Copy link
Contributor Author

shun2wang commented Aug 29, 2024

yeah, you can also find it from any failed actions: https://github.com/jasp-stats/jasp-desktop/actions/runs/10598393408

old packages has build problems with gcc version maybe.

@RensDofferhoff
Copy link
Contributor

Interesting! I have not seen these issues on my machines

@boutinb The updates are alright


loadModules();
}

void RibbonModel::addRibbonButtonModelFromDynamicModule(Modules::DynamicModule * module)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right way to do this. The loadModules(I) will recreate the modules, make some memory leakage (_entriesInsert, _entriesDelete, etc...), this is quite dangerous I think.
I don't understand why the 'special ribbon entries' are not translated, and the module ribbon buttons are. I remember that @JorisGoosen knew why. Maybe it is better to wait for his return of his holidays.

@JorisGoosen
Copy link
Contributor

Thanks for making this @shun2wang but Ive made another PR for this at #5653
That one obviates the need for destroying and recreating the buttons.
And also doesnt lead to possible problems with (re)loading the modules etc. Which could lead to problems with development modules, it also feels a bit more straightforward this way.

@JorisGoosen JorisGoosen closed this Sep 4, 2024
@shun2wang shun2wang deleted the fixLanguageHotUpdate branch September 4, 2024 12:34
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.

4 participants