Update dropshadows to the latest Kolibri Design System guidelines

[Suraj-kumar00]

Summary

This PR updates the dropshadow styles in the codebase to align with the latest Kolibri Design System guidelines.
I used only %dropshadow-1dp, %dropshadow-2dp, and %dropshadow-6dp to update the dropshadow from %dropshadow-4dp, %dropshadow-8dp, and %dropshadow-16dp.

References

Related issue: #12552
Kolibri Design System guideline: Dropshadow Spec

Reviewer guidance

• Inspect UI components using the updated dropshadow styles.
• Review that the dropshadow application aligns with the design system’s latest guidelines.

---

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical user journeys are covered by Gherkin stories
• Critical and brittle code paths are covered by unit tests

PR process

• PR has the correct target branch and milestone
• PR has 'needs review' or 'work-in-progress' label
• If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
• If this is an important user-facing change, PR or related issue has a 'changelog' label
• If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

• PR is fully functional
• PR has been tested for accessibility regressions
• External dependency files were updated if necessary (yarn and pip)
• Documentation is updated
• Contributor is in AUTHORS.md

[Suraj-kumar00]

Hi @MisRob, I opened the draft pull request for asking:

1. Like I got the issue and understood but how do I know in the various components which dp I should use 1dp, 2dp, or 6dp, it would be beneficial if you could help me to understand this.
2. Can you tell me what type of commit message I should use for good practice I haven't found it in the contribution guidelines.
3. And yeah, I'm doing these changes in the develop branch as per the docs is it good, or should I use any custom branch name like dropshadow-update?

[MisRob]

Hi @Suraj-kumar00, thanks! So from the code, you grasped the task overall, so all good in that regard. To answer

1. See the issue's link to the updated KDS guidelines in the "Elevation and shadow > Drop shadows" documentation section. Scroll down a bit and you will find concrete examples of components for each of the dropshadow values. If you encounter some components that you're unsure about, just ask here and I will clarify with the design team.
2. Thanks for asking, that's thoughtful. We don't really adhere to any standard. It's always good to follow general good practices such as clear messages and focused commits.
3. Yes, it's correct to target develop branch, and it's a good idea to work in a feature branch dropshadow-update. Name doesn't really matter to us, but rather for you. Later on, if you will need to rebase and similar, I think feature branch will make git operations easier.

[github-actions]

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0.dev0_git.20241021152522.pex
Windows Installer (EXE)	kolibri-0.18.0.dev0+git.20241021152522-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0.dev0+git.20241021152522-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0.dev0+git.20241021152522.dmg
Android Package (APK)	kolibri-0.18.0.dev0+git.20241021152522-0.1.4-debug.apk
TAR file	kolibri-0.18.0.dev0+git.20241021152522.tar.gz
WHL file	kolibri-0.18.0.dev0+git.20241021152522-py2.py3-none-any.whl

[MisRob]

Thanks @Suraj-kumar00! Would you fill in the PR template before I invite someone for review?

[Suraj-kumar00]

Hi @MisRob, how have you been?
Have you reviewed the PR? Let me know if any changes are required.

[MisRob]

Hi @Suraj-kumar00, I will invite reviewers. I'm sorry for delay - I am not getting notifications about PR template update, better to ping us next time :)

[MisRob]

@marcellamaki @akolson would you please review some time this week?

[marcellamaki]

Hi @Suraj-kumar00 - thanks very much for your work on this! And I apologize for the delayed review. I had done the review last week and then never saved my comments.

I added a few small requests for adjustments, but overall the work looks great. There are some scenarios which are either not covered by the documentation or are ambiguous, so I've tried to make some best-guesses for the sake of consistency. If you would be able to make those changes, that would be great, and then we will go ahead and approve and merge 🎉

Thanks again for your contribution!

[marcellamaki]

Thank you @Suraj-kumar00 for your contributions and updates! Approved. The last thing to resolve before merge is you will see that there is a conflict. Please rebase your PR off of develop and resolve the conflict, and then we can merge. Thank you

[Suraj-kumar00]

Hi @marcellamaki, tried but I never saw the confilct!
Can you please guide me?

[marcellamaki]

Hi @Suraj-kumar00 -- it looks like you are up to date with origin/develop which I'm guessing is your own forked branch of Kolibri, based on the screenshot

You'll want to make sure you also have a remote set for the learning equality upstream, if you haven't done so already. This allows you to keep your fork up to date with the main learning equality repo develop that you are merging into with this PR.

One you have both origin (your fork) and upstream (the LE kolibri repo) you can

git fetch upstream develop
git rebase upstream develop

From there, you should be able to see the conflicts, in your terminal, and resolve them in your IDE.

[Suraj-kumar00]

Hi @Suraj-kumar00 -- it looks like you are up to date with origin/develop which I'm guessing is your own forked branch of Kolibri, based on the screenshot

You'll want to make sure you also have a remote set for the learning equality upstream, if you haven't done so already. This allows you to keep your fork up to date with the main learning equality repo develop that you are merging into with this PR.

One you have both origin (your fork) and upstream (the LE kolibri repo) you can

git fetch upstream develop
git rebase upstream develop

From there, you should be able to see the conflicts, in your terminal, and resolve them in your IDE.

Can you check now?

[Suraj-kumar00]

Hi @Suraj-kumar00 -- it looks like you are up to date with origin/develop which I'm guessing is your own forked branch of Kolibri, based on the screenshot

You'll want to make sure you also have a remote set for the learning equality upstream, if you haven't done so already. This allows you to keep your fork up to date with the main learning equality repo develop that you are merging into with this PR.

One you have both origin (your fork) and upstream (the LE kolibri repo) you can

git fetch upstream develop
git rebase upstream develop

From there, you should be able to see the conflicts, in your terminal, and resolve them in your IDE.

Hi @MisRob @marcellamaki, how have you been?
Is this PR now able to be merged?

[marcellamaki]

Hi again @Suraj-kumar00 -- in my review I missed that there were still a few places with %dropshadow-4dp which should have been updated to use 1, 2, or 6. This is why the build is breaking for the .whl file which is resulting in a failing check. I've indicated those with comments. Could you update each of these places to 2dp? thank you!

[akolson]

It looks like the build failures are related to learningequality/kolibri-design-system#725 being merged which is still in progress. So we might have to wait to merge it in once ready. Adding a little blocker here! Thanks @Suraj-kumar00 for the changes this far!

[AlexVelezLl]

Hey @akolson. I jumped into the code and I saw the failures are because the shadows are exposed by kolibri-design-system/lib/styles/definitions, so we need to import @import '~kolibri-design-system/lib/styles/definitions'; to be able to use the @extend %dropshadow-2dp; statement. In these fles we are using @extend %dropshadow-2dp; without importing the kds definitions:

• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/plugins/learn/assets/src/views/TopicsPage/TopicsPanelModal.vue#L109
• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/plugins/pdf_viewer/assets/src/views/SideBar/index.vue#L149
• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/packages/kolibri-common/components/SearchFiltersPanel/index.vue#L374

@Suraj-kumar00 Could you please add the import @import '~kolibri-design-system/lib/styles/definitions'; at the top of the styles block to fix the failure please? :)

[Suraj-kumar00]

Hey @akolson. I jumped into the code and I saw the failures are because the shadows are exposed by kolibri-design-system/lib/styles/definitions, so we need to import @import '~kolibri-design-system/lib/styles/definitions'; to be able to use the @extend %dropshadow-2dp; sentence. In these fles we are using @extend %dropshadow-2dp; without importing the kds definitions:

• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/plugins/learn/assets/src/views/TopicsPage/TopicsPanelModal.vue#L109
• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/plugins/pdf_viewer/assets/src/views/SideBar/index.vue#L149
• https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/packages/kolibri-common/components/SearchFiltersPanel/index.vue#L374

@Suraj-kumar00 Could you please add the import @import '~kolibri-design-system/lib/styles/definitions'; at the top of the styles block to fix the failure please? :)

Sure @AlexVelezLl, So I have to add this import in all of the three file you mentioned right?

[AlexVelezLl]

Yes! @Suraj-kumar00 👐

[Suraj-kumar00]

@import '~kolibri-design-system/lib/styles/definitions';

One last thing should at the top:

Or here:

[AlexVelezLl]

In the <style> block @Suraj-kumar00. Here is an example https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/core/assets/src/views/BottomNavigationBar.vue#L115

[Suraj-kumar00]

In the <style> block @Suraj-kumar00. Here is an example https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/core/assets/src/views/BottomNavigationBar.vue#L115

Thanks updating soon.

[Suraj-kumar00]

In the <style> block @Suraj-kumar00. Here is an example https://github.com/Suraj-kumar00/kolibri/blob/2dbced371d59eeb798ae9f4851e6737041f9a1b4/kolibri/core/assets/src/views/BottomNavigationBar.vue#L115

Hi @AlexVelezLl, I have updated the changes. Let me know if there any other changes required.

Changes addressed

[AlexVelezLl]

LGTM! There are no more failing checks and code changes makes sense, I hanvent found any of the removed dropshadow values. Thank you @Suraj-kumar00!!!