Skip to content

Replace Material Icons with Kolibri Design System Icon #4913

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 21, 2025
Merged

Replace Material Icons with Kolibri Design System Icon #4913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55712c3
Replace Material Icons with Kolibri Design System Icon
SukhvirKooner Feb 14, 2025
1964e23
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Feb 17, 2025
fb3bb64
Update Details.vue
SukhvirKooner Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions contentcuration/contentcuration/frontend/shared/views/files/ImageOnlyThumbnail.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,17 @@
</div>
<svg
Copy link
Member

Choose a reason for hiding this comment

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

No update needed yet, just to understand the need to keep svg - if we removed svg and kept only KIcon, would there be a difference in user experience and what it would look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

  • Removing the <svg> makes the icon appear like in the screenshot, where it loses proper responsiveness and alignment.
  • Burying the icon within an <svg> ensures better adaptability since font-size scaling is harder to manage consistently.
  • Using <svg> provides better control over size, alignment, and responsiveness across different screen sizes and layouts.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @SukhvirKooner, that's helpful. The proper alignment should be achievable no matter of the presence of svg. However KIcon doesn't have auto-scaling feature as of now so it's fine to keep it withing svg to not have regressions.

We may want to consider having these behaviors supported by KIcon itself at some point in the future.

v-else
viewBox="0 0 24 24"
viewBox="0 0 40 40"
:aria-label="title"
class="nothumbnail-image"
:class="$isRTL ? 'rtl-image' : 'ltr-image'"
>
<text
<KIcon
icon="image"
:x="-1"
:y="y - 3"
:fill="$vuetify.theme.greyBorder"
class="material-icons notranslate v-icon"
>
image
</text>
:y="y - 14"
:style="{ fill: '#999999' }"
/>
</svg>

</figure>
Expand Down
26 changes: 13 additions & 13 deletions contentcuration/contentcuration/frontend/shared/views/files/Thumbnail.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,26 @@
:aria-label="kindTitle"
class="thumbnail-image"
>
<text
:x="-1"
:y="y"
fill="#ffffff"
class="material-icons notranslate v-icon"
>{{ icon }}</text>
<KIcon
icon="infoOutline"
:x="+10"
:y="y + 20"
:style="{ fill: '#ffffff' }"
/>
</svg>
<svg
v-else
viewBox="0 0 24 24"
viewBox="0 0 40 40"
:aria-label="kindTitle"
class="nothumbnail-image"
:class="$isRTL ? 'rtl-image' : 'ltr-image'"
>
<text
:x="-1"
:y="y - 3"
:fill="$vuetify.theme.greyBorder"
class="material-icons notranslate v-icon"
>image</text>
<KIcon
icon="image"
:x="-3"
:y="y - 14"
:style="{ fill: '#999999' }"
/>
</svg>

</figure>
Expand Down
4 changes: 2 additions & 2 deletions contentcuration/contentcuration/templates/base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@
<!-- Navigation bar -->
{% if INCIDENT %}
<div id="emergency-banner" class="text-center main-banner {% if INCIDENT.readonly %}error-banner{% else %}warning-banner{% endif %}">
<i class="material-icons">error</i> {{INCIDENT.message | safe}}
{{INCIDENT.message | safe}}
</div>
{% elif DEPRECATED %}
<div class="text-center main-banner warning-banner" id="redirect-banner">
<i class="material-icons">error</i> {% blocktrans %}Contentworkshop.learningequality.org has been deprecated. Please go to <a href="https://studio.learningequality.org">studio.learningequality.org</a> for the latest version of Studio{% endblocktrans %}
{% blocktrans %}Contentworkshop.learningequality.org has been deprecated. Please go to <a href="https://studio.learningequality.org">studio.learningequality.org</a> for the latest version of Studio{% endblocktrans %}
</div>
{% endif %}
{% endblock nav %}
Expand Down