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

changed jquery to safe mode in UI #1215

Merged
merged 15 commits into from
Mar 4, 2024
Merged

changed jquery to safe mode in UI #1215

merged 15 commits into from
Mar 4, 2024

Conversation

Greeshmanth1909
Copy link
Contributor

@Greeshmanth1909 Greeshmanth1909 commented Feb 23, 2024

resolve #1197
I've changed Jquery to Safe in the UI. Note that the s in safe mode is capitalized as it looked much cleaner than the uncapitalized version.

@Jaifroid I'll translate the same strings to French and Spanish once you are happy with the changes I've made in English.

@Greeshmanth1909 Greeshmanth1909 changed the title changed jquery to safe mode in UI (just english for now) changed jquery to safe mode in UI Feb 23, 2024
Copy link
Member

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

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

A good start, thanks! Please note that there will be a bunch of strings in app.js and uiUtil.js, and possibly others that will also need changing. These are, effectively, the strings in the json file that have IDs beginning with dialog-.

@Greeshmanth1909
Copy link
Contributor Author

Did a full repo search for JQuery mode, JQuery Mode and jquery mode, the only lines I found were either comments, actual code in automated tests or in .md files. I hope I didn't miss anything.

@Jaifroid
Copy link
Member

Thanks for this! I'll review and test properly when I get time, but for now I noticed the small issue above.

@Greeshmanth1909
Copy link
Contributor Author

What Spanish and French equivalent of "now deprecated" should I remove?

@Jaifroid
Copy link
Member

What Spanish and French equivalent of "now deprecated" should I remove?

Look for the words "obsoleto" (Spanish) and "obsolète" (French).

@Greeshmanth1909
Copy link
Contributor Author

The capitalization is a little off. Will fix it asap.

Greeshmanth1909 and others added 3 commits March 1, 2024 12:58
Copy link
Member

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

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

Thanks! Some small niggly details with the translations below.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 2, 2024

Also note that if viewing on GitHub, it hides some of the "conversations" above, and you have to click on "5 hidden conversation ... show more" to see all my suggested corrections.

Greeshmanth1909 and others added 4 commits March 3, 2024 21:14
Co-authored-by: Jaifroid <egk10@cam.ac.uk>
@Greeshmanth1909
Copy link
Contributor Author

Sorry for the delay, had a busy weekend.
Do note that I've removed (aka JQuery mode) in app.js as well.

Copy link
Member

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

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

It's looking good, thank you very much! There are the very very last things listed below. While you're doing those, I'll test the UI manually in my browser in the different languages. Then we'll be ready to merge!

@Jaifroid
Copy link
Member

Jaifroid commented Mar 4, 2024

Manual tests in French and Spanish are all good!

@Greeshmanth1909
Copy link
Contributor Author

@Jaifroid I've made the requested changes and hopefully, we didn't miss any minor details. Please go ahead and merge the changes once you're satisfied with them.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 4, 2024

@Jaifroid I've made the requested changes and hopefully, we didn't miss any minor details. Please go ahead and merge the changes once you're satisfied with them.

Great, it all looks good to me. Thank you very much for this PR. I've been unsatisfied for a long time with the name "JQuery Mode", so your original suggestion of "Safe Mode", and this PR, fix an irritation, and also give this mode a new purpose. I'll merge now.

@Jaifroid Jaifroid merged commit b77b385 into kiwix:main Mar 4, 2024
6 checks passed
@Greeshmanth1909
Copy link
Contributor Author

Happy to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename "JQuery mode" to "safe mode"?
2 participants