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

Highlight when mentioned extends to the bottom disproportionately. #3513

Open
ishammahajan opened this issue Jun 5, 2019 · 0 comments · May be fixed by #3514
Open

Highlight when mentioned extends to the bottom disproportionately. #3513

ishammahajan opened this issue Jun 5, 2019 · 0 comments · May be fixed by #3514

Comments

@ishammahajan
Copy link
Collaborator

3512-1

Here, as you can see, the highlight extends to the bottom by 1rem (value found in code) and top by 0. It should extend 0.5rem to the top and the bottom, otherwise the effect is definitely noticeable and quite jarring once you notice it.

ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Jun 5, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, by some CSS which might be confusing to
a reader.

With `.message + .message` we change the top and bottom padding of
messages which come directly after messages. This causes the spacing
to get messed up in the case where a header comes right after a
message which succeeds another message (so message --> message -->
header), it becomes 0.5rem instead of the default/previous 1rem.

`.message + .message + .header` selector fixes this issue by
introducing a top-margin to such a header. This method appears to be
hacky but this is the cleanest way such an issue could be fixed
without a huge change in the entire CSS.

Fixes zulip#3513.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Jun 6, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, through some CSS which might be
confusing to a reader.

The changes in CSS removes all application of padding on the highest
level of the markup (just below the `body`), and consolidates it to
the top.
Rule wise:
     1. All elements which don't have class message get margin.
     2. All message elements get padding of 8px vertically.
     3. All brief messages get an extended left padding of 64px.
     4. Because of the previous changes there are two cases where
     the behavior is not mimiced. One, which this rule fixes, is
     where a message-full directly follows any other message.

Second, which should not be fixed is where the distance between a
timerow and a message which precedes or succeeds it was 1.5rem
(24px), making the total empty space because of it (if it happened
to be in between two messages) 3rem (48px)! This should not be the
case design wise and this commit essentially sneak fixes that.

Fixes zulip#3513.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Jun 18, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, through some CSS which might be
confusing to a reader who is already familiar with the current
codebase.

The changes in the parent commit removes all application of padding
on the highest level of the markup (just below the `body`), and
consolidates it to the top.
Rule wise:
     1. All elements which don't have class message get margin.
     2. All message elements get padding of 8px vertically.
     3. All brief messages get an extended left padding of 64px.
     4. Because of the previous changes there are two cases where
     the design is not exactly the same from before this
     commit. One, which this rule (number 4) fixes, is where a
     message-full directly follows any other message. (The
     bug' was that there appears a loss of 24px of `padding-top`.)

Second, which should not be fixed is where the distance between a
timerow and a message which precedes or succeeds it was 1.5rem
(24px), making the total empty space because of it (if it happened
to be in between two messages) 3rem (48px)! This should not be the
case design wise and this commit essentially sneak fixes that.

Fixes zulip#3513.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Jun 18, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, through some CSS which might be
confusing to a reader who is already familiar with the current
codebase.

The changes in the parent commit removes all application of padding
on the highest level of the markup (just below the `body`), and
consolidates it to the top.
Rule wise:
     1. All elements which don't have class message get margin.
     2. All message elements get padding of 8px vertically.
     3. All brief messages get an extended left padding of 64px.
     4. Because of the previous changes there are two cases where
     the design is not exactly the same from before this
     commit. One, which this rule (number 4) fixes, is where a
     message-full directly follows any other message. (The
     bug' was that there appears a loss of 24px of `padding-top`.)

Second, which should not be fixed is where the distance between a
timerow and a message which precedes or succeeds it was 1.5rem
(24px), making the total empty space because of it (if it happened
to be in between two messages) 3rem (48px)! This should not be the
case design wise and this commit essentially sneak fixes that.

Fixes zulip#3513.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Aug 15, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, through some CSS which might be
confusing to a reader who is already familiar with the current
codebase.

The changes in the parent commit removes all application of padding
on the highest level of the markup (just below the `body`), and
consolidates it to the top.
Rule wise:
     1. All elements which don't have class message get margin.
     2. All message elements get padding of 8px vertically.
     3. All brief messages get an extended left padding of 64px.
     4. Because of the previous changes there are two cases where
     the design is not exactly the same from before this
     commit. One, which this rule (number 4) fixes, is where a
     message-full directly follows any other message. (The
     bug' was that there appears a loss of 24px of `padding-top`.)

Second, which should not be fixed is where the distance between a
timerow and a message which precedes or succeeds it was 1.5rem
(24px), making the total empty space because of it (if it happened
to be in between two messages) 3rem (48px)! This should not be the
case design wise and this commit essentially sneak fixes that.

There are also two more peculiar anomalies which are introduced at
the extremes of the message list. At the very top of the message
list some extra padding exists because the topmost element will
always be a `timerow`. And, the bottom most element will have a
reduced padding -- because of the fact that we're changing the
bottom padding of all message elements by 8px (16px -> 8px), and the
bottom most element will always be a message.

Fixes zulip#3513.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Aug 24, 2019
Mentions highlighting extends to the bottom and not at all at the
top. This commit fixes that, through some CSS which might be
confusing to a reader who is already familiar with the current
codebase.

Because of the changes made in this commit there are a few cases
where the design is not exactly the same as before this commit. One,
which `.message + .message-full` rule temporarily fixes, is where a
message-full directly follows any other message. (The bug' was that
there appears a loss of 24px of `padding-top`.)

Second, and third, are introduced at the extremes of the message
list. At the very top of the message list some extra padding exists
because the topmost element will always be a `timerow`. And, the
bottom most element will have a reduced padding -- because of the
fact that we're changing the bottom padding of all message elements
by 8px (16px -> 8px), and the bottom most element (which has
padding) will always be a messages.

Fixes zulip#3513.
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 a pull request may close this issue.

1 participant