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

Fix avatar upload prompt/tooltip floating wrong and permissions #5526

Merged
merged 4 commits into from
Jan 11, 2021
Merged

Fix avatar upload prompt/tooltip floating wrong and permissions #5526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4190cc
Fix mini avatar uploader "working" even without permissions
t3chguy Dec 27, 2020
8ee6525
Unfloat the MiniAvatarUploader "tooltip"
t3chguy Dec 27, 2020
ce5b244
delint
t3chguy Dec 27, 2020
7af1471
delint2
t3chguy Dec 27, 2020
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
10 changes: 10 additions & 0 deletions res/css/views/elements/_MiniAvatarUploader.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ limitations under the License.
position: relative;
width: min-content;

// this isn't a floating tooltip so override some things to not need to bother with z-index and floating
.mx_Tooltip {
display: inline-block;
position: absolute;
z-index: unset;
width: max-content;
left: 72px;
top: 0;
}

&::before, &::after {
content: '';
position: absolute;
Expand Down
20 changes: 14 additions & 6 deletions src/components/views/elements/MiniAvatarUploader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ limitations under the License.
*/

import React, {useContext, useRef, useState} from 'react';
import {EventType} from 'matrix-js-sdk/src/@types/event';
import classNames from 'classnames';

import AccessibleButton from "./AccessibleButton";
import Tooltip from './Tooltip';
import MatrixClientContext from "../../../contexts/MatrixClientContext";
import {useTimeout} from "../../../hooks/useTimeout";
import Analytics from "../../../Analytics";
import CountlyAnalytics from '../../../CountlyAnalytics';
import RoomContext from "../../../contexts/RoomContext";

export const AVATAR_SIZE = 52;

Expand Down Expand Up @@ -50,6 +51,11 @@ const MiniAvatarUploader: React.FC<IProps> = ({ hasAvatar, hasAvatarLabel, noAva

const label = (hasAvatar || busy) ? hasAvatarLabel : noAvatarLabel;

const {room} = useContext(RoomContext);
const canSetAvatar = room?.currentState.maySendStateEvent(EventType.RoomAvatar, cli.getUserId());
if (!canSetAvatar) return <React.Fragment>{ children }</React.Fragment>;

const visible = !!label && (hover || show);
return <React.Fragment>
<input
type="file"
Expand Down Expand Up @@ -82,11 +88,13 @@ const MiniAvatarUploader: React.FC<IProps> = ({ hasAvatar, hasAvatarLabel, noAva
>
{ children }

<Tooltip
label={label}
visible={!!label && (hover || show)}
forceOnRight
/>
<div className={classNames("mx_Tooltip", {
"mx_Tooltip_visible": visible,
"mx_Tooltip_invisible": !visible,
})}>
Comment on lines +91 to +94
Copy link
Member

Choose a reason for hiding this comment

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

it's a bit of a smell if we're ripping classes out of a component just to use them here. Can we make it a render mode of the Tooltip to do the work for us?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could but it'd make it an even more confusing component and we need to rebuild out Tooltips altogether as it is

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit torn because this is the sort of thing that we should probably avoid making worse, but the situation is so bad that it's on a shortlist of options.

@jryans might be best to review this

Copy link
Collaborator

Choose a reason for hiding this comment

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

The issues are quite annoying visually, so I think it's better to accept this change and then revisit tooltips holistically later.

<div className="mx_Tooltip_chevron" />
{ label }
</div>
</AccessibleButton>
</React.Fragment>;
};
Expand Down