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 time placeholder showing on mobile format for condensed messages #1442

Merged
merged 1 commit into from
Aug 23, 2017

Conversation

astorije
Copy link
Member

@astorije astorije commented Aug 23, 2017

/Cc @MaxLeiter

As an extra, make sure time placeholder cannot be selected anymore, and do not do an extra call to tz helper when time is not relevant/displayed.

I tried to entirely remove time/from blocks, but because of flexbox layout, that broke message alignment...

Before After
screen shot 2017-08-22 at 20 12 38 screen shot 2017-08-22 at 20 13 58
screen shot 2017-08-22 at 20 13 04 screen shot 2017-08-22 at 20 13 45
screen shot 2017-08-22 at 20 13 15 screen shot 2017-08-22 at 20 13 34

@astorije astorije added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Aug 23, 2017
@astorije astorije added this to the 2.5.0 milestone Aug 23, 2017
@MaxLeiter
Copy link
Member

MaxLeiter commented Aug 23, 2017

👍 tested and can confirm it works

<div class="msg {{type}} closed" data-time="{{time}}">
<span class="time hide-text">{{tz time}}</span>
<div class="msg condensed closed">
<span class="time">00:00</span>
Copy link
Member

Choose a reason for hiding this comment

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

This will break layout with seconds enabled

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'm stupid. Goddammit, I was so hoping this was the right better thing... So should I just leave actual time as is, for now? :/

Copy link
Member

Choose a reason for hiding this comment

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

Pretty much

Copy link
Member Author

Choose a reason for hiding this comment

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

:rage4:

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, fixed. Well, at least you can't select it by accident anymore...

As an extra, make sure time placeholder cannot be selected anymore, and do not do an extra call to `tz` helper when time is not relevant/displayed.

I tried to entirely remove `time`/`from` blocks, but because of flexbox layout, that broke message alignment...
@astorije astorije merged commit 91ecd99 into master Aug 23, 2017
@astorije astorije deleted the astorije/fix-condensed-time branch August 23, 2017 06:48
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
…ed-time

Fix time placeholder showing on mobile format for condensed messages
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
Development

Successfully merging this pull request may close these issues.

3 participants