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

Fixes filters not displayed on the imported exercises from QA channel #12935

Merged

Conversation

AllanOXDi
Copy link
Member

@AllanOXDi AllanOXDi commented Dec 11, 2024

Summary

This PR fixes filters not displaying if exercise is imported

Before

390492219-f4848053-14e6-4b67-9b0c-af3668fc2fd9.mp4

After

Screen.Recording.2024-12-11.at.14.26.21.mov

References

closes #12890

Reviewer guidance

Import the exercises from the QA Channel (nakav-mafak) and go to the Library page then the filter panel.

Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AllanOXDi - I confirm that the filters are working correctly now!

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor thing to completely fix the regression.

@@ -21,94 +21,93 @@
v-if="windowIsLarge || !currentCategory"
:class="windowIsLarge ? '' : 'drawer-panel'"
>
<div v-if="Object.keys(availableLibraryCategories).length">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this check is not being persisted anywhere.

If you look at the original PR where this was moved, it originally was used to wrap the section containing the categories, so that if no categories are in the local library, the section does not get displayed at all.

:value="value.keywords || ''"
@change="val => $emit('input', { ...value, keywords: val })"
/>
<h2 class="section title">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This <h2> and the next <div> should be wrapped in another div that is conditionalized using v-if="Object.keys(availableLibraryCategories).length".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

Copy link
Member Author

@AllanOXDi AllanOXDi Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was wrong

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Break what? The issue was that the condition was being applied to the entire filters panel previously, so when you were importing resources that had no categories set on them, that was causing the reported issue (that they happened to be exercises was a red herring, I think). Restoring the condition to its correct place just wrapping the categories title and div will restore the previous behaviour.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No- I was wrapping it wrongly . It's all good now.

@AllanOXDi AllanOXDi requested a review from rtibbles December 12, 2024 13:23
@AllanOXDi AllanOXDi force-pushed the filters-are-not-filtering branch from 707c36f to 2d704b1 Compare December 12, 2024 13:25
@@ -61,54 +61,55 @@
@click="handleCategory(key)"
/>
</div>
</div>
<div
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry - this div needs to be inside too, as we don't need to show the uncategorized button when there are no categories, because then, by definition, everything is uncategorized!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@AllanOXDi AllanOXDi force-pushed the filters-are-not-filtering branch from a8411f1 to c9f4bcd Compare December 12, 2024 15:25
@AllanOXDi AllanOXDi requested a review from rtibbles December 12, 2024 15:29
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working as expected now!

@rtibbles rtibbles merged commit d397ff7 into learningequality:develop Dec 12, 2024
36 of 37 checks passed
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.

0.18 - Library - The filters are not displayed if I have imported exercises
3 participants