Skip to content

Toggle learning translations in reading frame #27

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

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

Conversation

pirtleshell
Copy link

Hello,

It's nice to see a more-alive fork of this project. (Especially with so many imrovements!) I added the option to toggle the showing of translations under words with learning level 1. The checkbox allows you to hide them. It does this by adding a setting to the settings table under the key showlearningtranslations.

image

When on:

image

When off:

image

Uses set_text_mode.php:

image

I removed trailing spaces in the files I changed. They are in separate commits so you can more easily see what's going on by looking at the individual differences between commits.

@shaund
Copy link

shaund commented May 20, 2018

@pirtleshell There's no issues on your fork, else I would have put this there, though I guess it's relevant here too. I pulled your mine branch and am running it, but the Learning Translations checkbox isn't doing anything. If I uncheck it, nothing happens, and when I refresh the page, it's checked again.

I checked the database and saw showlearningtranslations wasn't added to the settings. I've manually added it, and now translations are hidden if I set it to 0 and shown if set to 1, but the checkbox isn't changing the values.

Also #26 is still present in your fork. It doesn't happen in https://github.com/lwt-project/lwt

@shaund
Copy link

shaund commented May 21, 2018

Not sure what happened, but it's working now.

@pirtleshell
Copy link
Author

Haha, hi @shaund. Did you manage to get it all working? I haven't used this in over a year so it's very likely that the code could have diverged greatly enough to have broken some things.

I'm also not sure what the state of this repo is. I made this PR because at the time this seemed to be a more active fork than the original. I also appreciated the speedups and decreased database size as well as all the other general enhancements. However, there doesn't seem to be any unified work being done on the Learning With Texts software... 😞

Now that the original is being more actively updated, it might be worthwhile to migrate some of the improvements made here to the original and see if we can get a more concerted dev effort on the project.

@argovela
Copy link

argovela commented May 23, 2018 via email

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.

3 participants