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

Fix left positioned tooltips being wrong and offset by fixed value #7551

Merged
merged 8 commits into from
Jan 19, 2022
Merged

Fix left positioned tooltips being wrong and offset by fixed value #7551

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
33407c5
Fix tooltip left position being wrong and offset by fixed value
MadLittleMods Jan 15, 2022
c559e90
Remove debug logging
MadLittleMods Jan 15, 2022
6c6e246
Adjust spacing to be more similar to before
MadLittleMods Jan 15, 2022
deb1f30
Fix read receipt tooltip not positioning correctly
MadLittleMods Jan 15, 2022
511849b
Update test snapshot
MadLittleMods Jan 15, 2022
ffece09
Adjust vertical offset to be centered
MadLittleMods Jan 17, 2022
ee6f3f8
Beter vertical center
MadLittleMods Jan 17, 2022
dca8d2e
Merge branch 'develop' into madlittlemods/fix-tooltip-positions
MadLittleMods Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion res/css/views/elements/_Tooltip.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ limitations under the License.
font-weight: 500;
Copy link
Contributor Author

@MadLittleMods MadLittleMods Jan 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I can't remove the element-web reviewer? I don't want to assign for review until ready.

Seems to be locked 🤔

Copy link
Member

Choose a reason for hiding this comment

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

not sure why - was this opened as a draft from the start or converted to a draft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was created normally. And I see the system note for converting to draft here:

Copy link
Contributor Author

@MadLittleMods MadLittleMods Jan 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it's something to do with the code owner but I don't think I did anything to trigger that (edit: maybe not, seems to be the same on other PRs). Usually it assigns element-web but I can remove it.

max-width: 200px;
word-break: break-word;
margin-right: 50px;
margin-left: 4px;
margin-right: 4px;

background-color: #21262C; // Same on both themes
color: $accent-fg-color;
Expand Down
8 changes: 6 additions & 2 deletions src/components/views/elements/Tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export default class Tooltip extends React.Component<ITooltipProps> {

private updatePosition(style: CSSProperties) {
const parentBox = this.parent.getBoundingClientRect();
console.log('updatePosition', 'alignment', this.props.alignment, 'parentBox', parentBox);
let offset = 0;
if (parentBox.height > MIN_TOOLTIP_HEIGHT) {
offset = Math.floor((parentBox.height - MIN_TOOLTIP_HEIGHT) / 2);
Expand All @@ -110,27 +111,30 @@ export default class Tooltip extends React.Component<ITooltipProps> {
offset = Math.floor(parentBox.height - MIN_TOOLTIP_HEIGHT);
}
const width = UIStore.instance.windowWidth;
console.log('tooltip width', width)
const parentWidth = (
this.props.maxParentWidth
? Math.min(parentBox.width, this.props.maxParentWidth)
: parentBox.width
);
const baseTop = (parentBox.top - 2 + this.props.yOffset) + window.pageYOffset;
const top = baseTop + offset;
const right = width - parentBox.right - window.pageXOffset - 16;
const left = parentBox.right + window.pageXOffset + 6;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing arbitrary offsets - 16 and + 6

const right = width - parentBox.left - window.pageXOffset;
Copy link
Contributor Author

@MadLittleMods MadLittleMods Jan 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

AFAICT, previously this was a logic mistake. Although it worked well enough for all of the icons around the app.

We have the whole width of the viewport, then want to subtract the left position of the element in question to get the space to the right to position the tooltip at.

const left = parentBox.right + window.pageXOffset;
const horizontalCenter = (
parentBox.left - window.pageXOffset + (parentWidth / 2)
);
switch (this.props.alignment) {
case Alignment.Natural:
if (parentBox.right > width / 2) {
console.log('natural left');
style.right = right;
style.top = top;
break;
}
// fall through to Right
case Alignment.Right:
console.log('right');
style.left = left;
style.top = top;
break;
Expand Down