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

Use correct variable name #71

Closed
wants to merge 1 commit into from
Closed

Use correct variable name #71

wants to merge 1 commit into from

Conversation

FWDekker
Copy link

@FWDekker FWDekker commented Apr 5, 2022

Fixes #58.

On lines 38/39 of index.js, the TOC's default state is set using the checkedState variable, which is passed to toc_entry.ejs. However, toc_entry.ejs reads the checked variable instead. This PR fixes that issue by using checkedState instead in toc_entry.ejs.

@flberger
Copy link

flberger commented Aug 10, 2022

This does not fix it. checked is passed as per

eejs.require('./templates/toc_entry.ejs', {checked: checkedState}, module);

in index.js.

The actual reason for #58 is the urlContainstocTrue check in postAceInit.js. It calls getParam in toc.js, which checks for &toc=false in the URL, and if that's not there, unconditionally enables the toc, not regarding the setting.

@FWDekker
Copy link
Author

I've updated the PR description to no longer say that it fixes #58. I'll leave the PR up because it does still fix an error.

@SamTV12345 SamTV12345 force-pushed the master branch 2 times, most recently from 1487fbd to 73a71ae Compare August 3, 2023 20:02
@SamTV12345
Copy link
Member

args.content +=
eejs.require('./templates/toc_entry.ejs', {checked: checkedState}, module);
Is this still relevant. An object with checked is passed. So that should not be an issue.

@SamTV12345
Copy link
Member

I force pushed the branch just in case we need to merge it.

@FWDekker
Copy link
Author

It does indeed seem like this PR does not solve any issues or bugs, so I'll close it.

@FWDekker FWDekker closed this Aug 29, 2023
@FWDekker FWDekker deleted the patch-1 branch August 29, 2023 14:53
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.

Config option disable_by_default not working
3 participants