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

Add ability to remove avatars #3719

Closed
wants to merge 8 commits into from
Closed

Add ability to remove avatars #3719

wants to merge 8 commits into from

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Dec 11, 2019

Signed-off-by: Michael Telatynski <[email protected]>
… t3chguy/remove_avatar

� Conflicts:
�	src/components/views/settings/ProfileSettings.js
Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
@t3chguy t3chguy requested a review from a team December 11, 2019 22:49
Signed-off-by: Michael Telatynski <[email protected]>
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

it also looks like the design implies that the context menu goes overtop of the box rather than to the right of it

if (disabled) return;

if (e.keyCode === KeyCode.ENTER || e.keyCode === KeyCode.SPACE) {
onChange(!checked);
e.stopPropagation();
e.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

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

unrelated changes should probably be in their own PR for next time

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

import sdk from "../../../index";

// TODO: Why is rendering a box with an overlay so complicated? Can the DOM be reduced?
const AvatarSetting = (props) => {
Copy link
Member

Choose a reason for hiding this comment

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

ooi why a function with a bunch of bootstrap instead of a tiny PureComponent class?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ability to use useContextMenu hook simplifies things

@t3chguy
Copy link
Member Author

t3chguy commented Dec 16, 2019

Superseded by #3733

@t3chguy t3chguy closed this Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard Accessibility of Room/Profile Avatar Upload No way to remove avatars from user/room settings
2 participants