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.

Sign up for GitHub

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

Jump to bottom

Room Tile context menu, notifications, indicator and placement #4858

Merged
merged 13 commits into from
Jul 1, 2020
Merged

Room Tile context menu, notifications, indicator and placement #4858

merged 13 commits into from
Jul 1, 2020

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Jun 29, 2020 •
edited
Loading

Fixes element-hq/element-web#14226
Fixes element-hq/element-web#14255
Fixes element-hq/element-web#13961

TODO:

  • Fix content of Notifications context menu
  • Wire up Notifications context menu
  • Use correct bell SVG
  • Moves context menus to the right places (aligns icons)
  • Adds hover states to their interactive elements
  • Flattens DOM to simplify later A11Y work
  • Simplifies some padding to match Figma (some were changed to make the hover state logical)
  • Adds and wires up Notifications context menu on Room Tile

image
image
image

@t3chguy
Fix RoomTile2 Context Menu to match Figma
adf186f 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Add room notifications context menu and non-default indicator to Room…
5441987 
…Tile2

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Add svgs
6b2ba8c 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@turt2live
Copy link
Member

For element-hq/element-web#13635

@t3chguy
Merge branches 'develop' and 't3chguy/room-list/2' of github.com:matr…
fe4cf9f 
…ix-org/matrix-react-sdk into t3chguy/room-list/2
@t3chguy
Fix User context menu alignment to match Figma
2a12fd1 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Redo Iconized Context Menu styling to match Figma and simplify future…
cbd2a9c 
… a11y work by flattening the DOM

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Iterate to match figma
198958d 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Wire up Notifications context menu on room tile 2
508dea1 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
turt2live
turt2live approved these changes Jun 30, 2020
View reviewed changes
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.

initial pass looks overall fine to me.

res/css/views/rooms/_RoomTile2.scss Show resolved Hide resolved
src/components/structures/UserMenu.tsx Show resolved Hide resolved
src/components/views/rooms/RoomTile2.tsx Outdated Show resolved Hide resolved
@t3chguy t3chguy mentioned this pull request Jun 30, 2020
Closed
2 tasks
@t3chguy
Iterate PR based on Figma design and feedback
b501428 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
align context menus even better
3b7b6ec 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy requested a review from turt2live July 1, 2020 13:07
@t3chguy t3chguy mentioned this pull request Jul 1, 2020
Closed
@t3chguy
add missing svg
0cbc506 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…
10e5475 
… t3chguy/room-list/2
@t3chguy
Hide notifications menu from invite tiles
9831698 
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy t3chguy marked this pull request as ready for review July 1, 2020 16:01
@t3chguy t3chguy merged commit 28e4300 into develop Jul 1, 2020
@t3chguy t3chguy deleted the t3chguy/room-list/2 branch July 1, 2020 18:20
MadLittleMods added a commit that referenced this pull request Feb 1, 2022
@MadLittleMods
Fix vertical spacing in compact context menus
4dc9ef9 
Fix element-hq/element-web#20801

Regressed in #7339

Relevant styles were first added in #4858
(context behind why the original styles were added)
@MadLittleMods MadLittleMods mentioned this pull request Feb 1, 2022
Merged
MadLittleMods added a commit that referenced this pull request Feb 1, 2022
@MadLittleMods
Fix vertical spacing in compact <ContextMenu> (#7684)
9b0da55 
Fix element-hq/element-web#20801

Regressed in #7339

Relevant styles were first added in #4858
(context behind why the original styles were added)

---

## Cause

Battling CSS specificity between the default and compact styles, https://specificity.keegan.st/

Known good (On `app.element.io` (expected)):
```css
// 0 3 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton {
    padding-top: 12px;
    padding-bottom: 12px;
}

// Compact styles override our default rules because they come
// after the other styles (source order) and have the same specificity
// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```

Bad (On `develop` (broken)):
```css
// Default rules always override because they have higher specificity.
// The `:not()` selector doesn't add any extra specificity but the selectors inside the `:not(...)` do.
// 0 4 0
.mx_IconizedContextMenu .mx_IconizedContextMenu_optionList .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
    padding-top: 12px;
    padding-bottom: 12px;
}

// 0 3 0
.mx_IconizedContextMenu.mx_IconizedContextMenu_compact .mx_IconizedContextMenu_optionList > * {
    padding: 8px 16px 8px 11px;
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turt2live turt2live turt2live approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@t3chguy @turt2live