Style of navbar fixed for wikitonary

[bakshiutkarsha]

Fixes #1033

[codecov]

Codecov Report

Merging #1251 into master will decrease coverage by 0.09%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1251      +/-   ##
==========================================
- Coverage   68.80%   68.71%   -0.10%     
==========================================
  Files          25       25              
  Lines        2199     2199              
  Branches      420      420              
==========================================
- Hits         1513     1511       -2     
- Misses        514      515       +1     
- Partials      172      173       +1     

Impacted Files	Coverage Δ
src/util/articleRenderers.ts	70.00% <0.00%> (-2.23%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfd2f36...ea41cce. Read the comment docs.

[kelson42]

This need an in detail explanation why we need to insert custom CSS. Why the CSS of the online version is not sufficient?

[bakshiutkarsha]

This need an in detail explanation why we need to insert custom CSS. Why the CSS of the online version is not sufficient?

As per the discussion on the issue thread and based on my investigation, it seems that either the event handler for toggling the visibility of the navframe is not getting attached or the custom CSS inserted by mwoffliner is causing problems with the visibility. To fix tthe issues, I tried both the approaches mentioned on the issue thread:

• Using javascript: I added a custom JS script to be bundled and loaded in the article page to remove client-js class from body and add the client-nojs class to it. This solution doesn't seem to fix the visibility issue.

• So finally, I had to retort to adding custom CSS that overrides the existing CSS for the NavFrame. This fixes the issue and the translations are visible as the custom CSS is marked important and would always override conflicting CSS property for this CSS class. The chances of it breaking something else is very low as the CSS is applied on just 1 property of 1 class.

[kelson42]

@bakshiutkarsha We should retrieve any css/js from the online version. If not we need to understand exactly why. This explanation is missing here... you fix a problem which is not even understodd priperly. This should be avoided.

[bakshiutkarsha]

@bakshiutkarsha We should retrieve any css/js from the online version. If not we need to understand exactly why. This explanation is missing here... you fix a problem which is not even understodd priperly. This should be avoided.

#1033 (comment)
@kelson42 Now, I carefully scrutinized all the comments by @Jaifroid on this ticket and this last comment helped me to resolve it. His previous comments have explained exactly the question of why is it happening and how to resolve it. I made the same fix suggested by him. Let me know if you think this fix is not proper, I will try to dig deeper.

[Jaifroid]

I'd be happy to test / troubleshoot the implementation here, but it may take a few days.

[kelson42]

No, like I said many times, I don’t want dependence to the wiki content in mwoffliner code.

[Jaifroid]

@bakshiutkarsha I'm not commenting on the general idea here, but only on your use of indexOf. The indexOf method returns -1 if the string you are searching does not contain the search string, not 0. Therefore, unless I've misunderstood what you're trying to do, I think you need:

~oneSection.text.indexOf('class="NavFrame"')

Note the tilde at the start. This converts -1 to 0, and 0 to -1, etc. Therefore, if the text appears at position 0, the tilde converts 0 to -1 and the test will pass. But if indexOf returns -1 (not found), the tilde converts it to 0, and the test fails.

[Jaifroid]

PS It would probably be better to use the classList.contains() method for this test. It is supported on IE10+. See https://dev.caniuse.com/classlist .

[Jaifroid]

Also, your test would fail if an editor had written class='NavFrame' with single quotes or class="dark NavFrame", hence why using classList.contains() is safer.

[kelson42]

@bakshiutkarsha Any news here? If our section mgmt code css+js is not able to make the difference between our section code and the content code, then we need to specify our DOM nodes properly, like for example by putting class="_mwoffliner"?

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.