search

z1dev / zkanji

Japanese language study suite and dictionary
GNU General Public License v3.0
59 stars 10 forks source link

BUG -- Kanji Search Widget: Filter-bug(s) #12

Closed am2del closed 6 years ago

am2del commented 6 years ago

Build (UTC): 2018-01-13 @ 12:12

Filter bug 1: Filter #3 (Meaning)

Reproduction steps: --> Hide all filters. --> Show #3 (Meaning). --> Fill in something. --> No filtering is performed until both of filter #1 & #2 are visible as well. NOTE: This seems to be affecting, at least, filter #3 (Meaning) and #4 (Reading), but - due to insufficient testing performed, this MAY affect any bug.

Filter bug 2: Filter #4 (Reading)

Reproduction steps: --> Hide all filters. --> Show #4 (Reading). --> Fill in a single Kana character. --> No filtering is performed due to bug mentioned above. --> Show filters #1, #2 & #3. (This due to bug mentioned above.) --> Filter #4 will now affect the search and show any Kanji which has a reading which starts with Kana. ----> Example: か --> will show か*, e.g.: 下（カ）、上（かみ）、川（かわ）、日（カ）...

More thorough testing is required! More filters may be affected and I haven't personally tested filters #5~#8 yet.

Proposal: Make a loop for filters which goes along the lines of:

for f in FILTERS
    if f is VISIBLE do
        apply FILTER f
        NEXT
    else do
        treat f as DISABLED    # DISABLED:  E.g. VALUE=NULL
        NEXT
done

This will ensure only visible filters is taken into account, even without resetting them.

Hidden filters should be treated as DISABLED as the user may forget to take them into account when browsing the result. (E.g. forgotten to press "Reset" or simply wants to keep the filter set to go back to previous search etc.)

z1dev commented 6 years ago

The fix is uploaded on git, please check now. If it's working, I'll close this issue.

It was supposed to work the way you described: when a filter is hidden it's disabled. What must have broken this was moving the JLPT filter in front of the reading and meaning filters, although there might have been other things that broke this before that. There was also a bug in the filtering on JLPT, which is now fixed as well.

am2del commented 6 years ago

Build (UTC): 2018-01-15 @ 15:06

I can confirm this bug fixed. (Or at least so it appears, only did a quick test due to lack of time.)

...however - I got some feedback (which I agree with) regarding to two specific filters. Gonna open a feature-request for it as it may require minor additions.

z1dev commented 6 years ago

I'll close this issue. If another one comes up with the kanji search widget, please open a new one.