Skip to content

Conversation

akolson
Copy link
Member

@akolson akolson commented Sep 3, 2025

Summary

This pr fixes unresponsiveness of the trash page Restore button and Add New Folder button in the move resource page caused by the css overriding done when fixing #5322. To fix the issue, a new prop appearanceOverrides has been added to the KTooltip component so its appearance can be better manipulated, particularly the z-index in this case.

Note: Testing this pr is dependent on merging and releasing learningequality/kolibri-design-system#1112

Before:

Restore button

restore.mp4

Add New Folder button

new.folder.mp4

After:
The tooltips still work as before

Screen.Recording.2025-09-03.at.18.38.35.mov

Restore button

Screen.Recording.2025-09-03.at.18.44.29.mov

Add New Folder button

Screen.Recording.2025-09-03.at.18.46.27.mov

Note:
Please note that this a11y contrast issue has been fixed as part of this pr.

References

Fixes #5350
Fixes #5349

Reviewer guidance

  • Go to a channel and move or delete a folder or a resource.
  • Do one of the following depending on the above;
    • Select the "Open trash" option from the ellipsis button and attempt to restore the deleted folder or resource.
    • For the move option, Click the "Add new folder" to create a folder to move the resource or folder.
  • Both operations should work as expected.

@akolson akolson changed the title [DO NOT MERGE] Fix css override troubles Fix css override troubles Sep 5, 2025
@akolson akolson marked this pull request as ready for review September 5, 2025 17:30
@akolson akolson requested a review from bjester September 8, 2025 13:36
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

There are other tooltips that had the same issues within the modals to begin with, so this only targets a subset-- the recommendation cards. So I feel this PR perpetuates the problematic z-indices and doesn't fully resolve some of the problems either.

The first contributing issue is that the trash and move modals have embedded modals, which is a bad practice to begin with. Moving the those outside helps a little and prepares us better for the Vuetify migration. After solving the former, a solution is to simply have those components toggle between the two modals instead of keeping both open at the same time. Having these modals in the routing could also be a solution, but either way, the solutions handle the modal deficiencies that should be resolved regardless.

@bjester bjester self-assigned this Sep 9, 2025
@akolson
Copy link
Member Author

akolson commented Sep 12, 2025

@bjester, the latest commit should resolve the issue—thanks for the recommendations. Flattening the embedded modals restored button clickability, but the MessageDialogs were not displaying as expected (due to z-index troubles). To fix this, I migrated the usages of MessageDialog to KModal, which fully resolved the issue.

@akolson akolson force-pushed the fix-css-override-troubles branch from 29dfd4f to dfa4d79 Compare September 12, 2025 09:25
@akolson akolson force-pushed the fix-css-override-troubles branch from dfa4d79 to e83ef5d Compare September 12, 2025 09:29
@akolson akolson requested review from bjester and pcenov and removed request for pcenov September 12, 2025 09:34
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

The nested modals and dialogs are working as expected. The changes seem completely reasonable to me. LGTM! Thanks for handling these regressions @akolson

@marcellamaki marcellamaki merged commit 4aa3274 into learningequality:hotfixes Sep 16, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants