Skip to content

Use standard CSS rules to resize emoticons #29276

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
rahimrahman merged 1 commit into from
Dec 5, 2024
Merged

Use standard CSS rules to resize emoticons #29276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 18 additions & 4 deletions webapp/channels/build/emoji/make_emojis.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,14 @@ const cssRules = `.emojisprite-preview {
background-repeat: no-repeat;
cursor: pointer;
-moz-transform: scale(0.5);
transform: scale(0.5);
Copy link
Contributor

@rahimrahman rahimrahman Dec 4, 2024

Choose a reason for hiding this comment

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

@lorumic Maybe we can do both here? Do you mind testing on both?

Suggested change
transform: scale(0.5);
zoom: 0.5; /* fixes blurry images on Chrome? */
transform: scale(0.5); /* fixes small images on Firefox? */

Copy link
Contributor Author

@lorumic lorumic Dec 4, 2024

Choose a reason for hiding this comment

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

The problem is that if you do both, both get applied on FF 133, resulting in the issue that was reported: 0.5 from zoom and 0.5 from transform: scale results in a final 0.25 size.
I think the takeaway here is that the usage of zoom should not be mixed with that of transform: scale. You could use zoom only, but that would probably affect your browser compatibility: scale vs zoom.

Copy link
Contributor

@rahimrahman rahimrahman Dec 4, 2024

Choose a reason for hiding this comment

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

Ah thank you. I acknowledge that my knowledge of css is subpar, thus I'm seeking others to help. I'm just throwing a few ideas.

Perhaps we can use @supports? Again, this may not work as I don't spend much time on web css (I'm more of a mobile engineer here at MM).

.zoomable {
  zoom: 0.5; /* Default for browsers supporting zoom */
}

@supports (transform: scale(1)) {
  .zoomable {
    zoom: unset; /* Reset zoom for modern browsers */
    transform: scale(0.5); /* Use scale instead */
    transform-origin: 0 0; /* Anchor scaling */
  }
}

Copy link
Contributor Author

@lorumic lorumic Dec 4, 2024

Choose a reason for hiding this comment

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

This is a good idea, but it should be the other way around, as zoom is the less-supported property. :)
I have just updated the code to use zoom whenever supported (which means "always" in Chrome, where the blurriness problem originally occurred, because Chrome supports it since version 1), and fall back to transform: scale when unsupported (e.g. FF versions before 126).
Can you maybe have another look @rahimrahman?

Copy link
Contributor

@rahimrahman rahimrahman Dec 4, 2024

Choose a reason for hiding this comment

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

Smart, totally see your reasoning of reversing. Of course, my example would have cause blurriness on Chrome and totally not fixing the problem. 😏

Do you have a screenshot of Chrome which shows no blurriness and another on Firefox that shows the icon is the correct size? Visual representation helps a lot, especially for other reviewers.

Thanks for working to solve this even at small scale (pun intended!). I appreciate helping me understand the diff between zoom and scale. Something I have not encountered before (again as a mobile dev, I don't spend a lot of time here).

Copy link
Contributor

@rahimrahman rahimrahman Dec 4, 2024

Choose a reason for hiding this comment

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

Just re-read that you're on Linux. Let me check on mine, or we can get one of our QAs to verify.

transform-origin: 0 0;
// Using zoom for now as it results in less blurry emojis on Chrome - MM-34178
zoom: 0.5;

@supports (zoom: 0.5) {
-moz-transform: none;
transform: none;
zoom: 0.5;
}
}

.emojisprite {
Expand All @@ -453,7 +458,7 @@ const cssRules = `.emojisprite-preview {
border-radius: 18px;
cursor: pointer;
-moz-transform: scale(0.35);
zoom: 0.35;
transform: scale(0.35);
}

.emojisprite-loading {
Expand All @@ -465,7 +470,16 @@ const cssRules = `.emojisprite-preview {
border-radius: 18px;
cursor: pointer;
-moz-transform: scale(0.35);
zoom: 0.35;
transform: scale(0.35);
}

@supports (zoom: 0.35) {
.emojisprite,
.emojisprite-loading {
-moz-transform: none;
transform: none;
zoom: 0.35;
}
}

${cssCats.join('\n')}
Expand Down
22 changes: 18 additions & 4 deletions webapp/channels/src/sass/components/_emojisprite.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
background-repeat: no-repeat;
cursor: pointer;
-moz-transform: scale(0.5);
transform: scale(0.5);
transform-origin: 0 0;
// Using zoom for now as it results in less blurry emojis on Chrome - MM-34178
zoom: 0.5;

@supports (zoom: 0.5) {
-moz-transform: none;
transform: none;
zoom: 0.5;
}
}

.emojisprite {
Expand All @@ -18,7 +23,7 @@
background-repeat: no-repeat;
cursor: pointer;
-moz-transform: scale(0.35);
zoom: 0.35;
transform: scale(0.35);
}

.emojisprite-loading {
Expand All @@ -30,7 +35,16 @@
background-repeat: no-repeat;
cursor: pointer;
-moz-transform: scale(0.35);
zoom: 0.35;
transform: scale(0.35);
}

@supports (zoom: 0.35) {
.emojisprite,
.emojisprite-loading {
-moz-transform: none;
transform: none;
zoom: 0.35;
}
}

.emoji-category-recent { background-image: url('images/emoji-sheets/apple-sheet.png'); }
Expand Down
17 changes: 14 additions & 3 deletions webapp/channels/src/sass/components/_emoticons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,12 +548,23 @@ $emoji-footer-height: $emoji-footer-border-width + $emoji-half-height + $emoji-

img {
-moz-transform: scale(0.45);
zoom: 0.45;
transform: scale(0.45);

@supports (zoom: 0.45) {
-moz-transform: none;
transform: none;
zoom: 0.45;
}

&.emoji-category--custom {
-moz-transform: scale(1);
transform: scale(1.25);
zoom: 1;
transform: scale(1);

@supports (zoom: 1) {
-moz-transform: none;
transform: none;
zoom: 1;
}
}
}
}
Expand Down
Loading