Fix used counter for Filter and presets #2431
Conversation
sudeeptarlekar
force-pushed
the
bug/2430/use_counter_in_presents
branch
2 times, most recently
from
85f6a0f to
b99af87
Compare
When user imports the counter in the Chipmunk, we show the counter which indicates how many times imported filter is used. When user clicks on the counter then counter keeps increasing even if filter is already applied. This commits fix that by checking if filter is applied or not, if filter is used then, counter will not be increamented. Fixes: esrlabs#2430
sudeeptarlekar
force-pushed
the
bug/2430/use_counter_in_presents
branch
from
b99af87 to
82d4f8b
Compare
There was a problem hiding this comment.
I've tested that on my machine. and I think the current behavior isn't entirely correct, because now we are getting that the number of used filters is set to zero even though the filter is currently in use (See screenshot). Then after I click on the preset the counter will get into 1 and then stop increasing.
This is easily reproduceable by just applying a filter or opening a session with saved filter in it
|
Also please check why the lint CI is failing |
When user loads the file and collection is already loaded, counter for collection is set to 0, this commit fixes that bug.
sudeeptarlekar
force-pushed
the
bug/2430/use_counter_in_presents
branch
from
b7286dd to
da79032
Compare
There was a problem hiding this comment.
Thanks for the fixes. I've added some comments regarding the code changes and here are some points form testing the changes:
- We need to think about highlighting the used one because the current color doesn't affect the readability of the texts. Can you consider leaving the background color the same but make the title of used filters bold instead?
- I ran into situation when I imported filters and set list filter to
Allwhere some presets have(0)and others don't have it
| if (def.used > 0) { | ||
| def.used = 0; | ||
| } |
There was a problem hiding this comment.
I think we can just set the value to zero without having an if branch.
| ); | ||
| } | ||
|
|
||
| // TODO: Remove this function as this is not used anywhere |
There was a problem hiding this comment.
We can resolve the TODO then by removing the function
| collection.last = Date.now(); | ||
| } | ||
|
|
||
| public ignoreAll() { |
There was a problem hiding this comment.
I think we need rename this to something more precise like resetUsedCounters.
|
@sudeeptarlekar We need to consider this whole approach because the counter of the presets is globally for all open tabs (See screenshot). In that case, the used counter should be allowed to have values more than 1 and we need actually to increase it. I would suggest to fix increasing the counter on mouse click and leave the current behavior as in master for this part. Maybe in a new PR? 
|
2 participants
When user imports the presets. Chipmunk shows number 1 next to the filename which suggests that filter is already been used. This commit fixes that bug.
Fixes: #2430