Skip to content

Conversation

@WinnyChang
Copy link
Member

@WinnyChang WinnyChang commented Jul 15, 2025

Summary

  • Applied relevant styles to the image element.
  • Added a custom expand icon to KDS and used it on the expand icon button.
  • Manually tested the hover and focus effects with both mouse and keyboard.

Screen Recordings - Mouse Hover Effects and Responsive Sizing

Before

before-image-styling-mouse-and-responsive.mov

After

after-image-styling-mouse-and-responsive.mov

Screen Recordings - Tab Navigation

Before

before-image-styling-keyboard.mov

After

after-image-styling-keyboard.mov

References

Issue - #13504
Add expand icon PR - learningequality/kolibri-design-system#1068

Reviewer guidance

Check if:

  • The image is displayed with the specified margin, border, and alignment.
  • The image size is responsive and maintains its aspect ratio.
  • The “expand” icon button and image's drop shadow appear on hover.
  • The “expand” icon button is keyboard-focusable and appears on focus.

@github-actions github-actions bot added SIZE: medium DEV: renderers HTML5 apps, videos, exercises, etc. DEV: frontend labels Jul 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 15, 2025

@WinnyChang WinnyChang marked this pull request as ready for review July 21, 2025 19:16
@akolson akolson requested review from AllanOXDi and akolson July 21, 2025 19:16
const ALLOWED_URI_REGEXP = /^(?:(?:blob:https?|data):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i;
const FORBID_TAGS = ['style', 'link'];
const FORBID_ATTR = ['style'];
const FORBID_ATTR = ['style', 'width', 'height'];
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for forbidding the width and height attributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

The image in the test resource has a width=300 attribute, which overrides the defined image size in the style.scss file. Forbidding the width and height ensures full control over the image size, allowing it to follow the Figma design (width: 100%, max-width: 1200px, max-height: 80vh).

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thanks!

Copy link
Member

@akolson akolson left a comment

Choose a reason for hiding this comment

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

No further comments here. This should be good to merge!
Thanks @WinnyChang!

@akolson akolson merged commit a69be80 into learningequality:release-v0.18.x Jul 25, 2025
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DEV: frontend DEV: renderers HTML5 apps, videos, exercises, etc. SIZE: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants