Wanikani Open Framework [developer thread]

[Kumirei]

Seems to me that such a feature would be outside the scope of the framework. It’s a problem better solved with other scripts.

Also, thank you for the file cache methods. I am looking to store minified review data in IndexedDB for the heatmap and it makes things so much easier.

[Kumirei]

Suggestion: let settings inherit path properties.
To keep the Heatmap settings ordered, I override all setting paths. Although it’s not a big deal to add the path to all settings, it would be simpler if I could put the path on a higher level item. In this case it would be nice if I could just put path: '@lessons' in the page config, and it would automatically place the contained settings in that path.

[Kumirei]

@rfindley

Ahh… I see it’s not actually setting the value at that point, it’s only setting the default if it doesn’t exist yet.

The actual relevant code is:

if (typeof context.cfg.pre_open === 'function') context.cfg.pre_open(dialog);
context.reversions = $.extend(true,{},wkof.settings[context.cfg.script_id]);
refresh(context);

It’s using refresh() to populate the values after you make any initial changes in pre_open().

Should I also run refresh() before pre_open() if pre_open() is present?

Came upon this issue again with the new version of the Heatmap, haha. Good thing I could look back at this and be reminded why it wasn’t working as I expected. Ended up just wrapping my pre_open function and refreshing before it was called. A much better solution than what I did last time

[Kumirei]

@rfindley I am having an issue where the default value of a setting is interfering with the chosen value.

I am storing an array of arrays for the heatmap. These are the defaults

colors: [[0, "#AAAAAA"], [100, "#BBBBBB"], [200, "#CCCCCC"],],

If I change the setting (and save it) to be

colors: [[0, "#DDDDDD"], [100, "#EEEEEE"],],

When I load the settings again, with the defaults, they will end up like this

colors: [[0, "#DDDDDD"], [100, "#EEEEEE"], [200, "#CCCCCC"],],

Is this intended?

[rfindley]

This is the merge code:

wkof.settings[script_id] = $.extend(true, {}, defaults, settings);

It starts with an empty object {}, then merges the defaults, then merges the loaded settings.

So, I’m guessing this is how $.extend() works. I wouldn’t have expected it to merge parts of an array.

Edit:
On a 'deep' extend, Object and Array are extended, but object wrappers on primitive types such as String, Boolean, and Number are not. Deep-extending a cyclical data structure will result in an error.

So, the deep merge (“true”) parameter is apparently causing the array to merge. But the deep merge is needed for nested object parameters. I’m not sure what solution to suggest, other than removing the color default and checking if it needs to be added after the merge.

[UInt2048]

How does the extend() function work in jQuery?

Excerpt:

By this we can see that jQuery.extend():

• Starts with the object provided by the first parameter.
• Adds to it any property in the second parameter. If the property already exists in the first parameter, it is overwritten. The object for the second parameter is unchanged.
• Repeats the above with any subsequent parameter.
• Returns the first parameter.

This is useful for combining user and default option-objects together to get a complete set of options:

[Kumirei]

@rfindley

So, the deep merge (“true”) parameter is apparently causing the array to merge. But the deep merge is needed for nested object parameters. I’m not sure what solution to suggest, other than removing the color default and checking if it needs to be added after the merge.

Ah, thought it could be something like that. A bit annoying but your workaround will do. Thank you or looking into it!

[Kumirei]

Suggestion: Invalidated setting values should stop settings from saving

[prouleau]

I have this bug. I am not sure it belongs here so feel free to redirect me.

My list of scripts falls off the screen and the scrolling arrow too. I can’t scroll and access all my scripts settings. Is this an issue with Open Framework? I have an horizontal scroll bar at the bottom of my screen. Could this be the issue?

[UInt2048]

@prouleau

My list of scripts

…is too long? How do you have that many scripts with settings? end confusion

@prouleau I understand, my point was that it’s crazy that you found all those scripts…

[prouleau]

The number of scripts is not the issue. We should be able to scroll this list. There is a scrollbar for this (look at the screenshot) and I can’t use it. The scrollbar should be made to work.

[rfindley]

It is a framework issue. Apparently, the framework is a victim of its own success

I don’t have time to work on it currently, but maybe someone with some CSS skill could take a quick look. If someone finds a solution, I’ll gladly add it to the framework.

[UInt2048]

@rwesterhof

The settings use the open framework’s method of pushing it into the menu, and that doesn’t support min/max settings on the input fields (we could put in a feature request on that script of course).

Feature request: allow this - min/max on number input

This can be useful for keeping settings reasonable (no -1 reviews or lessons)

[lunapg]

I see one major issue : you don’t have any test nor CI. This is madness.
You really need to think about it, or you will have trouble in fixing regression bug,

[prouleau]

@rfindley

I don’t have time to work on it currently, but maybe someone with some CSS skill could take a quick look. If someone finds a solution, I’ll gladly add it to the framework.

The fix: add this line at the end of the main css file.

.sitemap__expandable-chunk>*:first-child .sitemap__expandable-chunk>*:first-child {max-height:330px;

The the dropdown will look like this:

It is much shorter than before but is very easy to use. It scrolls very nicely.

[rfindley]

@UInt2048

Feature request: allow this - min/max on number input

Open Framework does have an optional validation function for inputs in the Settings dialogs, and you can return a message to be displayed when the input is invalid.

[UInt2048]

That’s good to know - it still might be useful to add the min and max support on number inputs - even though it’s no substitute for validation (you never know what sort of wacky browser you are dealing with, e.g. IE9)

[seanblue]

I don’t think userscripts work in IE anyway.

[rfindley]

It does have min/max on numbers (see below). And I think it has built-in messages if it’s out of range.

GitHub - rfindley/wanikani-open-framework: Open framework for writing...

[prouleau]

@rfindley the fix I had posted has not been published in the official version yet. Is there any reason for this?

Just noticed: I didn’t paste the closing brace in my fix. A correction:

.sitemap__expandable-chunk>*:first-child .sitemap__expandable-chunk>*:first-child {max-height:330px;}

This goes at the end of:jqui-wkmain.css