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

Add crossmark symbol sym.rs #4113

Merged
merged 2 commits into from May 13, 2024
Merged

Add crossmark symbol sym.rs #4113

merged 2 commits into from May 13, 2024

Conversation

giannissc
Copy link
Contributor

No description provided.

@Enivex
Copy link
Collaborator

Enivex commented May 10, 2024

I would suggest changing these to mark.cross and mark.check

@giannissc
Copy link
Contributor Author

Ready! 🙂 @Enivex

@giannissc
Copy link
Contributor Author

Ready!

Copy link
Contributor

@Coekjan Coekjan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I am not the author/collaborator of the repo, so you still need to wait for their reviews.

@laurmaedje
Copy link
Member

@Enivex there is also refmark and servicemark further below, so changing checkmark to mark.check maybe makes that a little inconsistent? It's probably also a bit harder to find mark.check than checkmark.

@Enivex
Copy link
Collaborator

Enivex commented May 13, 2024

@Enivex there is also refmark and servicemark further below, so changing checkmark to mark.check maybe makes that a little inconsistent? It's probably also a bit harder to find mark.check than checkmark.

I don't think so. Reference mark and service mark seem to fulfil entirely different roles than a check mark, so I'm not sure they belong together. On the other hand, a cross mark and check mark definitely do.

I think most people should just search for "check" or "mark" anyway.

@laurmaedje
Copy link
Member

I didn't mean to imply that they belong together, more that mark is maybe a little to general as a parent for mark.check since there are so many other kinds of marks. So maybe checkmark and crossmark would be more consistent.

@Enivex
Copy link
Collaborator

Enivex commented May 13, 2024

I didn't mean to imply that they belong together, more that mark is maybe a little to general as a parent for mark.check since there are so many other kinds of marks. So maybe checkmark and crossmark would be more consistent.

They could be grouped under ballot instead?

@laurmaedje
Copy link
Member

The existing ballot symbols all have the box as part of the glyph, so I think that would also be a bit inconsistent. Naming symbols is really tricky ^^

@Enivex
Copy link
Collaborator

Enivex commented May 13, 2024

The existing ballot symbols all have the box as part of the glyph, so I think that would also be a bit inconsistent. Naming symbols is really tricky ^^

On the other hand, the Unicode name for the cross mark is ballot x 😛.

Feel free to do what you think is right. (It could always be changed later too.)

@laurmaedje
Copy link
Member

I think for the time being, I'd prefer the original checkmark / crossmark. lt and gt are also not grouped under a shared parent, which is sort of similar I guess.

@laurmaedje
Copy link
Member

Thanks!

@giannissc
Copy link
Contributor Author

Let me know if you would like any other changes! @laurmaedje , @Enivex

@laurmaedje
Copy link
Member

No, it's all good now. It will be merged automatically once CI has passed. :)

@laurmaedje laurmaedje added this pull request to the merge queue May 13, 2024
Merged via the queue into typst:main with commit e8ee152 May 13, 2024
6 checks passed
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.

None yet

4 participants