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

update emojis #127

Merged
merged 2 commits into from
Feb 17, 2024
Merged

update emojis #127

merged 2 commits into from
Feb 17, 2024

Conversation

sbfnk
Copy link
Contributor

@sbfnk sbfnk commented Feb 15, 2024

Closes #126

@lorenzwalthert
Copy link
Owner

lorenzwalthert commented Feb 15, 2024

@sbfnk can you please also update the snapshots of the tests to make the unit tests pass?

no_change <- "&nbsp;&nbsp;:ballot_box_with_check:"
no_change <- ":heavy_check_mark:"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Why the switch from ☑️ to ✔️? Aesthetic preference or is there a readability advantage or something like that?

Copy link
Contributor Author

@sbfnk sbfnk Feb 15, 2024

Choose a reason for hiding this comment

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

Purely aesthetic - not sure why but on (my) Chromium this gets rendered to a small thin rather than heavy checkmark:

image

But I don't feel strongly - happy to revert in fact.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I don't mind it. I might even prefer it? I was mainly asking because there is sometimes stuff that's better for not obvious but important reasons like screen-reader friendliness or something :)
My version ^^:
image

Copy link
Owner

@lorenzwalthert lorenzwalthert left a comment

Choose a reason for hiding this comment

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

Thanks.

@sbfnk sbfnk mentioned this pull request Feb 16, 2024
@lorenzwalthert
Copy link
Owner

I approved the changes, I think you could have merged yourself. I tend to let contributors merge their own PR.

@lorenzwalthert lorenzwalthert merged commit 6cbc622 into lorenzwalthert:main Feb 17, 2024
5 of 6 checks passed
@sbfnk sbfnk deleted the emojis branch February 17, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emojis no longer rendered properly in the comments
3 participants