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

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

Closed
am2del opened this issue Jan 13, 2018 · 3 comments
Closed

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

am2del opened this issue Jan 13, 2018 · 3 comments

Comments

@am2del
Copy link

am2del commented Jan 13, 2018

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
Copy link
Owner

z1dev commented Jan 15, 2018

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
Copy link
Author

am2del commented Jan 15, 2018

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
Copy link
Owner

z1dev commented Jan 22, 2018

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

@z1dev z1dev closed this as completed Jan 22, 2018
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

No branches or pull requests

2 participants