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

config: add scripts for Wiktionary's show/hide toggles #1665

Closed
wants to merge 1 commit into from

Conversation

zhuowei
Copy link
Contributor

@zhuowei zhuowei commented Oct 16, 2022

This fixes #1033.

The ext.gadget.VisibilityToggles and ext.gadget.defaultVisibilityToggles only seem to exist on Wiktionary: when I tested with English Wiktionary, they fixed the toggles. When I tested with English Wikipedia, the scripts returned mw.loader.state({ "ext.gadget.defaultVisibilityToggles": "missing" });

I can't find any good way to determine whether a wiki uses toggles: https://phabricator.wikimedia.org/T161278 seems to suggest that the API doesn't surface gadgets in modulescripts?

@ MananJethwani found that it's not possible to scrape them from RLPAGEMODULES either, at least through the API: https://www.mediawiki.org/wiki/Topic:W2g7t4u4whlmkqho

Is there a way to add these for Wiktionary only, maybe via Wiktionary's Zimfarm config instead? Or maybe pull the wiki's homepage via the regular, non-API endpoint and check for gadget scripts?

@kelson42
Copy link
Collaborator

@zhuowei Thank you for your PR, I will review it.

@kelson42 kelson42 self-requested a review October 16, 2022 08:11
@tacsipacsi
Copy link

Instead of importing a custom gadget from Wiktionary, I think you should import the standard module jquery.makeCollapsible, and convince English Wiktionary to use it. This will be present on any Wikimedia wiki, helping more users than importing the custom gadget. (Or maybe the standard module isn’t broken at all?) For example, Hungarian Wiktionary uses the standard module, see https://hu.wiktionary.org/wiki/Sablon:trans-top as an example (unfortunately I couldn’t test whether it works, as Kiwix doesn’t seem to work on my Android 12 phone at all).

@zhuowei
Copy link
Contributor Author

zhuowei commented Oct 16, 2022

Hungarian Wiktionary uses the standard module

I checked: Hungarian Wiktionary on Desktop shows the box fully expanded by default; tapping it closes it - the opposite of English Wiktionary.

On mobile, it always shows the box expanded without any way to collapse it: https://hu.m.wiktionary.org/wiki/világ%C3%ADtótorony

Kiwix does the same as mobile currently, and always shows the box expanded: https://library.kiwix.org/wiktionary_hu_all_nopic_2022-09/A/világ%C3%ADtótorony

I did look into adding a CSS override that would force the infobox open on English Wiktionary, but I'd rather get the show/hide toggle working if possible.

@tacsipacsi
Copy link

I checked: Hungarian Wiktionary on Desktop shows the box fully expanded by default; tapping it closes it - the opposite of English Wiktionary.

True, due to the low number of languages per article, I decided not to collapse them by default. But this is just a matter of configuration: adding the mw-collapsed class would make it collapsed by default.

On mobile, it always shows the box expanded without any way to collapse it: https://hu.m.wiktionary.org/wiki/világ%C3%ADtótorony

Oh yeah, maybe I should advocate for the standard module to work on mobile first…

Kiwix does the same as mobile currently, and always shows the box expanded: https://library.kiwix.org/wiktionary_hu_all_nopic_2022-09/A/világ%C3%ADtótorony

Does Kiwix use the mobile or desktop/default versions of modules (where this matters)? If the desktop/default one, importing jquery.makeCollapsible would add collapsibility on huwiktionary in Kiwix regardless of it not working on the mobile web.

@kelson42
Copy link
Collaborator

@zhuowei I still don’t have tested, but two remarks already:

  • Obviously it has to continue to work with other sites like Wikipedia without generating errors
  • Please add a comment in the code itself before the newly added modules, so we can understand the logic behind this… Maybe at some point the API will deliver the proper list and we will be able to remove this « custom » list!

@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Base: 68.92% // Head: 68.92% // No change to project coverage 👍

Coverage data is based on head (2c97de0) compared to base (2c97de0).
Patch has no changes to coverable lines.

❗ Current head 2c97de0 differs from pull request most recent head d2abf57. Consider uploading reports for the commit d2abf57 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1665   +/-   ##
=======================================
  Coverage   68.92%   68.92%           
=======================================
  Files          26       26           
  Lines        2394     2394           
  Branches      467      467           
=======================================
  Hits         1650     1650           
  Misses        580      580           
  Partials      164      164           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kelson42 kelson42 force-pushed the wiktionary-show-hide-script branch 2 times, most recently from 7630037 to 5e53d52 Compare December 13, 2022 20:25
@kelson42
Copy link
Collaborator

@zhuowei I have superseeded this PR with #1701 to be able to add a comment. Even if the fix is not the perfect solution, this seems indeed the very best thing to do and this will make a lot of happy people. Thank you very much for this PR.

@kelson42 kelson42 closed this Dec 31, 2022
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.

Wiktionary translations can't be shown
3 participants