Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fixes issue #21381 #8199

Closed
wants to merge 4 commits into from
Closed

Fixes issue #21381 #8199

wants to merge 4 commits into from

Conversation

goelesha
Copy link
Contributor

@goelesha goelesha commented Mar 30, 2022 •
edited by github-actions bot
Loading

Notes: Fix a bug (element-hq/element-web#21381) where I have setup min-height to avoid overlapping timestamps

Fixes: element-hq/element-web#21381

Here's what your changelog entry will look like:

🐛 Bug Fixes

@goelesha goelesha requested a review from a team as a code owner March 30, 2022 19:27
@robintown robintown added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Mar 31, 2022
germain-gg
germain-gg suggested changes Mar 31, 2022
View reviewed changes
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

I'm not sure this currently solves the issue.

Could you please post screenshots of the fix in the description of the PR? It would be beneficial to see the fix on the different timeline layouts too

src/components/views/messages/MessageTimestamp.tsx
@@ -48,6 +48,7 @@ export default class MessageTimestamp extends React.Component<IProps> {
className="mx_MessageTimestamp"
title={formatFullDate(date, this.props.showTwelveHour)}
aria-hidden={true}
min-height="1rem"
Copy link
Contributor

Choose a reason for hiding this comment

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

There are no min-height attributes on a span HTML element.

My hunch is that we would have to update the mx_EventTile_line CSS class rather than the message timestamp. It's positioned absolutely and therefore will not affect the height of the event tile

@goelesha goelesha closed this Mar 31, 2022
@goelesha goelesha deleted the issue#21381 branch March 31, 2022 09:05
@luixxiul
Copy link
Contributor

@goelesha you would not have to close the PR and delete the branch. You just could push additional commit(s) to address the question.

@goelesha
Copy link
Contributor Author

@goelesha you would not have to close the PR and delete the branch. You just could push additional commit(s) to address the question.

Ohh Okay. I thought as I was on the wrong path and understood the problem later. So I will create a separate PR.
Will keep this in mind for the future.

@goelesha goelesha restored the issue#21381 branch March 31, 2022 11:49
@goelesha goelesha reopened this Mar 31, 2022
@goelesha goelesha closed this Mar 31, 2022
@goelesha goelesha deleted the issue#21381 branch March 31, 2022 12:29
@goelesha
Copy link
Contributor Author

@goelesha you would not have to close the PR and delete the branch. You just could push additional commit(s) to address the question.

Ohh Okay. I thought as I was on the wrong path and understood the problem later. So I will create a separate PR. Will keep this in mind for the future.

The new PR can be viewed on this link.
#8205

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@germain-gg germain-gg germain-gg requested changes

Assignees
No one assigned
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Set min-height to event when formatted HTML is empty to avoid overlapping timestamps
4 participants
@goelesha @luixxiul @germain-gg @robintown