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

Jump lists app names #2280

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Jump lists app names #2280

wants to merge 9 commits into from

Conversation

patrickdalla
Copy link
Collaborator

@patrickdalla patrickdalla commented Aug 2, 2024

Identify Jumplist app name based on know list of IDs.

I put this in DRAFT because I used back tick to declare a var from a multiline literal (the list of know apps).
It worked only when adding "-Dnashorn.args=--language=es6". But this can affect other modules.
So, if it do affect any other modules, I will have to change it and declare this list in other way.

@lfcnassif
Copy link
Member

Thank you @patrickdalla, this can be very useful!

Seems to me #2278 has 4 identical commits to this PR. Should #2278 be closed without merging? By the way, ideally I think the new categories should be prepared for translation.

@patrickdalla
Copy link
Collaborator Author

patrickdalla commented Aug 3, 2024

Hi Nassif,
PR 2280 depends on 2278 commit, so it were also included.
I've made separate PR's because #2280 was incompatible with current js ES5 support, and maybe you prefer not to use another approach than a JS task to do the work, so you wouldn't approve.

But I refactored the JS to be ES5 compatible and if you approve the way it is implemented PR2280 can be merged and 2278 skipped and closed.

I will just finish the localization right now.

@patrickdalla patrickdalla marked this pull request as ready for review August 3, 2024 11:11
@patrickdalla
Copy link
Collaborator Author

patrickdalla commented Aug 3, 2024

ready.

If you think JS task approach is not the best way to implement this app id name resolution you can skip this pr and merge 2278.

@lfcnassif
Copy link
Member

Thank you @patrickdalla! Using a javascript task is fine.

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.

2 participants