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

Eliminate space around emoji #5474

Merged
merged 9 commits into from Nov 7, 2017
Merged

Eliminate space around emoji #5474

merged 9 commits into from Nov 7, 2017

Conversation

nullkal
Copy link
Contributor

@nullkal nullkal commented Oct 19, 2017

Example: https://edge.mstdn.jp/@nullkal/98856648358617597

Before:
image

After:
image

This PR eliminates space around emojis. We often use emojis like this, so I created this patch so that they are connected seamlessly.

@nullkal nullkal changed the title Eliminatespace around emoji Eliminate space around emoji Oct 19, 2017
@lynlynlynx
Copy link
Contributor

lynlynlynx commented Oct 19, 2017

I know its interesting to make looooong fox, but for common use of emojis, top and bottom paddings are needed.
ws000123
and padding-top for every status__content makes number of toots displayed fewer. Only for connecting emojos!!!😕 Ah, if there is only 2px for padding-top, I dont mind this.

However I think, removing left and right paddings is not bad, because a space is automatically inserted.

@nullkal
Copy link
Contributor Author

nullkal commented Oct 19, 2017

I think this PR is important because Slack has the same behavior (There is no space around custom emojis) and people importing custom emojis from Slack would be disappointed to look space around emojis they added.

I made a change not to modify unicode emoji's behavior. Because of that, this PR affects only custom emojis now. We can add spaces by editing image for custom emojis, so I think there is little concern about custom emojis.

image

And in regard to padding-top, how about decreasing status__action-bar's margin-top from 10px to 5px. This modification sets off the increase of the height (I already applied this modification in the above screenshot).

@lynlynlynx
Copy link
Contributor

lynlynlynx commented Oct 19, 2017

From a viewpoint of design, I cannot tolerate uneven styles (sizes, margins) between custom emojis and normal emojis (Fixed, thanks!). I cannot tolerate making general design worse ONLY for connecting emojos.
General administrators don't add connenting emojos, using/adding full-size square emojis is even more general. We frequently use general emojis in every toots, but how about connecting emojos? even if you are emojo lovers, you use them at most once a week or so, huh? Users can adjust horizontal gaps by themselves, but they cannot adjust vertical gaps.

It's my opinion, I’d like to ask other opinions.


And in regard to padding-top, how about decreasing status__action-bar's margin-top from 10px to 5px. This modification sets off the increase of the height.

It's not bad, thanks.

@nullkal
Copy link
Contributor Author

nullkal commented Oct 19, 2017

Other apps' behaviors

Slack

image

I set the Emoji Style to Twitter Style (Twemoji) for taking this screnshot.

Discord

image

Anyway, we need more people's opinions about this, because talk about 'General admins' without actually listen to them is meaningless.

@nullkal
Copy link
Contributor Author

nullkal commented Oct 19, 2017

When I took a screenshot of Slack I noticed that Slack has no space around not only custom emojis but also normal emojis. I think this behavior is better now, and reverted 6a5bdf0 and 54a8c60.

@nolanlawson nolanlawson added ui Front-end, design suggestion Feature suggestion requires in depth labels Oct 21, 2017
@noraworld
Copy link
Contributor

noraworld commented Oct 25, 2017

I like this because some users make a big emoji by concatenating them like looooong fox. Another approach I think is better is to emojify with no space, not to change the appearance. For example, Fox:fox::fox: is converted to Fox🦊🦊.

This is also convenient for those who don't prefer a space between a word and an emoji. Japanese has no space unlike English, so I often glue a word and an emoji together, e.g. かえる🐸, no space between them.

To emojify with no space is not easy because a shortcode is closed with two colons. In other words we must discriminate between first colon and last colon. But I have implemented it before in other project. I'll make a pull request if this approach is favored a lot and no one doesn't do.

I'd also like to ask your opinions.

@Gargron Gargron merged commit 3f16caa into mastodon:master Nov 7, 2017
yi0713 added a commit to yi0713/mastodon that referenced this pull request Nov 8, 2017
@Gargron
Copy link
Member

Gargron commented Dec 5, 2017

This PR introduced 5px extra space between status text and username which looks very odd. It really stands out when you switch between 2.0.0 and master frontends. Can that detail be fixed @nullkal @lynlynlynx ?

@nullkal
Copy link
Contributor Author

nullkal commented Dec 5, 2017

I added the space because emojis is cropped without it. Another solution is overflow: visible.

@lynlynlynx
Copy link
Contributor

This PR makes emojis as large as they overflow height of letters, so it is impossible to keep exactly the same as before.
I have two suggestions below, please give me your opinion @Gargron .

master suggestion 1
2px extra space
suggestion 2
0px extra space and move emojis 2px lower
ws000127 ws000128 ws000129

This PR reduces the margin between status text and action buttons from 10px to 5px. Should it be also restored? (I think 5px is enough, ummm)

@nullkal
Copy link
Contributor Author

nullkal commented Dec 5, 2017

Suggestion: add overflow: visible to .status__content, .reply-indicator__content:

2017-12-05 17 46 37

Is there any reasons to set overflow: hidden to it?

@lynlynlynx
Copy link
Contributor

overflow: visible revokes the margin between status text and username, it's not good.

ws000008

@Gargron
Copy link
Member

Gargron commented Dec 5, 2017

@lynlynlynx Both your suggestions (1, 2) look better than master to me. I think 2 is best. And yes I want the margin restored too...

@Gargron Gargron mentioned this pull request Dec 5, 2017
10 tasks
cobodo pushed a commit to cobodo/mastodon that referenced this pull request Dec 6, 2017
* Eliminate space around emoji

* More improve emoji style

* Make more compatible with Twemoji

* Make scss-lint happy

* Make not modify normal emoji's behavior

* Decrease status__action-bar's margin-top to 5px

* Make the test be passed

* Revert "Make the test be passed"

This reverts commit 54a8c60.

* Revert "Make not modify normal emoji's behavior"

This reverts commit 6a5bdf0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion Feature suggestion ui Front-end, design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants