Skip to content
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.

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

fix: Escape trademark sign #500

Merged
merged 3 commits into from
Jul 24, 2021
Merged

Conversation

2xB
Copy link
Contributor

@2xB 2xB commented Jul 15, 2021

This PR is moved from RocketChat/Rocket.Chat#21833 , following the moved code.


  • I have read the Contributing Guide
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have labeled the PR correctly with the related package
  • I have run Loki's visual regression tests (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Since this change is so small, repeating existing patterns and I currently don't have a test environment set up, I dared to not test it on my local machine and hope when reviewing this, it doesn't provide significant additional work to test this insignificant fix.

Proposed changes (including videos or screenshots)

Before this, the trademark character (™) was automatically replaced with ™️ , showing the trademark symbol in a totally unfitting size for most contexts. This commit avoids that by replacing the sign with its HTML escaped version before further replacements e.g. via emojione are applied (as can be seen in Rocket.Chat/client/lib/renderMessageBody.ts).

This is identical to how e.g. the registered trademark sign is currently handled.

Before fix:
image

Issue(s)

Fixes RocketChat/Rocket.Chat#7386.

Further comments

Escaping symbols like ® in escapeHTML already looks like a workaround around a bug in emojione/https://github.com/RocketChat/Rocket.Chat/blob/develop/app/emoji-emojione/lib/rocketchat.js for me, since I don't know of an other reason to escape these characters this way other than preventing emojione from picking them up. Since this is the approach that is already in use, I just continue this approach logically for one more symbol.


Pinging @MartinSchoeler as they implemented the initial version of the edited files in RocketChat/Rocket.Chat#19817 and therefore might be in a good position to provide input?

Before this, the trademark character (™) was automatically replaced with ™️ ,
showing the trademark symbol in a totally unfitting size for most contexts.
This commit avoids that by replacing the sign with its HTML escaped version
before further replacements e.g. via emojione are applied
(as can be seen in `Rocket.Chat/client/lib/renderMessageBody.ts`).

This is identical to how e.g. the registered trademark sign is currently handled.

Fixes RocketChat#7386.
@tassoevan tassoevan merged commit 801de02 into RocketChat:develop Jul 24, 2021
gabriellsh added a commit that referenced this pull request Jul 28, 2021
…into feat/createCloudWorkspaceForm

* 'develop' of github.com:RocketChat/Rocket.Chat.Fuselage:
  docs(github): Update pull request template (#504)
  fix: Escape trademark sign (#500)
  feat(onboarding-ui): AdminInfoPage for Cloud Registration wizard (#497)
gabriellsh added a commit that referenced this pull request Aug 13, 2021
…into new/image-manipulation

* 'develop' of github.com:RocketChat/Rocket.Chat.Fuselage: (23 commits)
  fix: modal styles
  feat(onboarding-ui): Create Cloud Workspace Form (#499)
  Pass style-loader output into babel-loader
  Check ES version after bundling Fuselage
  fix(onboarding-ui): Labels wrapping under checkbox (#509)
  chore(fuselage): Nested sidebar props type (#510)
  feat(onboarding-ui): Request trial form (#505)
  v0.28.0
  fix: Missing Sidebar type (#506)
  fix: Missing Sidebar type (#506)
  docs(github): Update pull request template (#504)
  fix: Escape trademark sign (#500)
  feat(onboarding-ui): AdminInfoPage for Cloud Registration wizard (#497)
  feat(onboarding-ui): Self-hosted registration (#501)
  fix: Add Tab.Item definition (#503)
  fix tabs.d.ts (#502)
  feat(onboarding-ui): Invalid Link page (#496)
  feat(onboarding-ui): Server registration form (#494)
  feat(onboarding-ui): Administrator information form and Organization information form (#489)
  fix: Forwarding ref in Select components (#492)
  ...
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.

Add option to not use ™️ icon for ™ character
2 participants