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 timestamp tooltips not aligning correctly with timestamps #1999

Merged
merged 2 commits into from
Jan 29, 2018

Conversation

astorije
Copy link
Member

@astorije astorije commented Jan 23, 2018

Fixes #1998.
Fixes #2000.

Very quick, in-browser attempt at fixing #1998. Not sure if this breaks anything else.

Before After
screen shot 2018-01-23 at 00 45 05 screen shot 2018-01-23 at 00 50 24

@astorije astorije added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Jan 23, 2018
@astorije
Copy link
Member Author

I ended up adding a commit for #2000 directly to this branch, because it's literally on the same line (all in the browser, didn't even fire my editor :successkid:).

Before After
screen shot 2018-01-23 at 00 52 22 screen shot 2018-01-23 at 01 06 02

No idea how I break stuff with this though. I see my second commit is kinda reverting 74ca130, which isn't cool. What about applying overflow: hidden to .msg .text instead of .msg?

(btw this is why I hate opening issues: I always end up trying to fix it even when I don't have the time to do so, instead of just recording it not to forget 😅)

@@ -1044,7 +1044,7 @@ kbd {
word-wrap: break-word;
word-break: break-word; /* Webkit-specific */
display: flex;
overflow: hidden;
Copy link
Member

Choose a reason for hiding this comment

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

Need to keep this rule, at least on .text. Otherwise stuff like zalgo text is going to overflow which we don't want.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I mentioned that in my previous comment. I moved it to .content (it had no effect on .text, I'm guessing because it's an inline span 🤷‍♂️).

Zalgo is now prevented again. Until we want to add a tooltip in a .content, for example a status message or something, and we'll be screwed again :D
But until then, that should fix it.

@astorije astorije force-pushed the astorije/fix-timestamp-tooltip branch from 006fe9d to 5722bd9 Compare January 24, 2018 05:22
@xPaw xPaw added this to the 3.0.0 milestone Jan 24, 2018
@astorije astorije merged commit 15ea2db into master Jan 29, 2018
@astorije astorije deleted the astorije/fix-timestamp-tooltip branch January 29, 2018 06:07
@xPaw xPaw modified the milestones: 3.0.0, 2.7.1 Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
2 participants