Replace all chevrons with a single icon by jryans · Pull Request #5067 · matrix-org/matrix-react-sdk

jryans · 2020-07-30T15:51:40Z

This replaces all chevrons with a single icon, affecting components like the
room list, network dropdown, and rebrand modal.

Rather than having files for each direction that can get out of sync, this
change uses one direction and rotates in CSS for the others.

Network dropdown

Rebrand modal

Room list

Jump to bottom

Jump to unread

With guides to confirm alignment

Fixes element-hq/element-web#14663

This replaces all chevrons with a single icon, affecting components like the room list, network dropdown, and rebrand modal. Rather than having files for each direction that can get out of sync, this change uses one direction and rotates in CSS for the others. Fixes element-hq/element-web#14663

benedikrok · 2020-07-30T16:06:19Z

@jryans

Show more should have the same chevron as "People" title for example
The network dropdown can be scaled up a bit and repositioned so it's vertically centered to the "X" icon above, and we can nudge it down a bit so it's horizontally aligned with the text 'now it's a it up)
Riot to Element is a surprise to me, since it's more of an illustrative composition than a regular UI. If we are re-using the same icon in this case, make it bigger and probably slightly thicker

jryans · 2020-07-30T16:43:36Z

Show more should have the same chevron as "People" title for example

Hmm, so the chevron next to People and next to Show more should be the same icon at the same size... Do you want both to be the same colour then? If so, which colour, or have I misunderstood entirely...? 😅

The network dropdown can be scaled up a bit and repositioned so it's vertically centered to the "X" icon above, and we can nudge it down a bit so it's horizontally aligned with the text 'now it's a it up)

Okay, I have made it larger to match the X icon above, and repositioned to align with text and X icon. Screenshot updated above.

Riot to Element is a surprise to me, since it's more of an illustrative composition than a regular UI. If we are re-using the same icon in this case, make it bigger and probably slightly thicker

Yeah, it just happened to be using the chevron icon previously, and I didn't really want to leave it around just for that screen, so hoping to adapt the new one to fit here as well. I made it bigger and thicker, screenshot updated above.

benedikrok · 2020-07-30T16:52:20Z

Show more should have the same chevron as "People" title for example

Hmm, so the chevron next to People and next to Show more should be the same icon at the same size... Do you want both to be the same colour then? If so, which colour, or have I misunderstood entirely...? 😅

Here is an example of how do they look like in context: https://www.figma.com/file/rHnynEgrePUtXVear8rGT0/34-Room-List?node-id=657%3A12676
Same size, same color (both light and dark theme), just different positioning

The network dropdown can be scaled up a bit and repositioned so it's vertically centered to the "X" icon above, and we can nudge it down a bit so it's horizontally aligned with the text 'now it's a it up)

Okay, I have made it larger to match the X icon above, and repositioned to align with text and X icon. Screenshot updated above.

Perfect sizing! I'd just move down a bit more and match the thickness of the lines with the "X" above.

Riot to Element is a surprise to me, since it's more of an illustrative composition than a regular UI. If we are re-using the same icon in this case, make it bigger and probably slightly thicker

Yeah, it just happened to be using the chevron icon previously, and I didn't really want to leave it around just for that screen, so hoping to adapt the new one to fit here as well. I made it bigger and thicker, screenshot updated above.

Looks great!

jryans · 2020-07-30T16:57:56Z

Here is an example of how do they look like in context: https://www.figma.com/file/rHnynEgrePUtXVear8rGT0/34-Room-List?node-id=657%3A12676
Same size, same color (both light and dark theme), just different positioning

Hmm, I think I am missing whether that's what you want or what is wrong... As far as I can tell, the size and position for both matches the Figma screen. The colour is currently different. Should I change the color to match Figma? Or is something wrong with the position? If so, you may need to be more verbose, as I can't tell what is different from Figma at the moment. 😅

benedikrok · 2020-07-30T17:04:19Z

Here is an example of how do they look like in context: https://www.figma.com/file/rHnynEgrePUtXVear8rGT0/34-Room-List?node-id=657%3A12676
Same size, same color (both light and dark theme), just different positioning

Hmm, I think I am missing whether that's what you want or what is wrong... As far as I can tell, the size and position for both matches the Figma screen. The colour is currently different. Should I change the color to match Figma? Or is something wrong with the position? If so, you may need to be more verbose, as I can't tell what is different from Figma at the moment. 😅

The show more seems smaller for some reason in the preview. The alignment looks fine, but the colors should be different.
I don't have a clue why colors are different but the Room sections, Show more and chevrons on both should be in the same color.

jryans · 2020-07-30T18:38:15Z

The show more seems smaller for some reason in the preview. The alignment looks fine, but the colors should be different.
I don't have a clue why colors are different but the Room sections, Show more and chevrons on both should be in the same color.

Thanks for clarifying. I have double-checked to verify, and the chevrons for room section and show more are the same size.

As for colours, there's actually several colours in Figma as well, so I am not sure which you want:

Figma

Both chevrons, room list section header text: #8D99A5 (tertiary text)
Show more text: #737D8C (secondary text)

The same is true in the dark theme version: show more text uses secondary, while the others use tertiary.

Current app without changes

Light theme

Show more text and chevron: #737D8C (secondary text)
Room list section header text and chevron: #2e2f32 (primary fg text)

Dark theme

Show more text and chevron: #A9B2BC (room tile preview)
Room list section header text: #8E99A4 (only used here)
Room list section header chevron: #ffffff (primary fg text)

@benedikrok, should everything be changed to tertiary text colours in both themes then...? Does that include the message previews on room tiles as well...?

benedikrok · 2020-07-30T18:55:42Z

The show more seems smaller for some reason in the preview. The alignment looks fine, but the colors should be different.

I don't have a clue why colors are different but the Room sections, Show more and chevrons on both should be in the same color.

Thanks for clarifying. I have double-checked to verify, and the chevrons for room section and show more are the same size.

As for colours, there's actually several colours in Figma as well, so I am not sure which you want:

Figma

Both chevrons, room list section header text: #8D99A5 (tertiary text)

Show more text: #737D8C (secondary text)

The same is true in the dark theme version: show more text uses secondary, while the others use tertiary.

Current app without changes

Light theme

Show more text and chevron: #737D8C (secondary text)

Room list section header text and chevron: #2e2f32 (primary fg text)

Dark theme

Show more text and chevron: #A9B2BC (room tile preview)

Room list section header text: #8E99A4 (only used here)

Room list section header chevron: #ffffff (primary fg text)

@benedikrok, should everything be changed to tertiary text colours in both themes then...? Does that include the message previews on room tiles as well...?

You're right, I forgot we made an exception for the show more text. Let's stick with the values we have in Figma. Sorry for the confusion 😬

turt2live

lgtm code-wise

turt2live · 2020-07-30T21:24:50Z

        width: 24px;
        height: 24px;
-        right: -28px; // - (24 + 4)
+        right: -26.5px; // - (24 + 2.5)


while we're here, can we reference what 24 and 2.5 mean?

bwindels · 2020-07-31T12:24:18Z

Nique was asking me to also replace the jump down chevron with this one as well during our design polish session, could you also use the new chevron here please 🙏?

.mx_JumpToBottomButton_scrollDown::before {
    content: "";
    position: absolute;
    top: 0;
    bottom: 0;
    left: 0;
    right: 0;
    mask: url('$(res)/img/icon-jump-to-bottom.svg');
    mask-repeat: no-repeat;
    mask-position: center;
    background: $muted-fg-color;
}

benedikrok · 2020-07-31T12:26:09Z

Nique was asking me to also replace the jump down chevron with this one as well during our design polish session, could you also use the new chevron here please 🙏?
.mx_JumpToBottomButton_scrollDown::before {
    content: "";
    position: absolute;
    top: 0;
    bottom: 0;
    left: 0;
    right: 0;
    mask: url('$(res)/img/icon-jump-to-bottom.svg');
    mask-repeat: no-repeat;
    mask-position: center;
    background: $muted-fg-color;
}

How does it look like?

bwindels · 2020-07-31T12:27:11Z

Nique was asking me to also replace the jump down chevron with this one as well during our design polish session, could you also use the new chevron here please pray?
.mx_JumpToBottomButton_scrollDown::before {
    content: "";
    position: absolute;
    top: 0;
    bottom: 0;
    left: 0;
    right: 0;
    mask: url('$(res)/img/icon-jump-to-bottom.svg');
    mask-repeat: no-repeat;
    mask-position: center;
    background: $muted-fg-color;
}
How does it look like?

See #5074

benedikrok · 2020-07-31T12:37:42Z

Nique was asking me to also replace the jump down chevron with this one as well during our design polish session, could you also use the new chevron here please pray?
.mx_JumpToBottomButton_scrollDown::before {
content: "";
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
mask: url('$(res)/img/icon-jump-to-bottom.svg');
mask-repeat: no-repeat;
mask-position: center;
background: $muted-fg-color;
}
How does it look like?

See #5074

Let's change it. Just make sure the sizing will make sense. I. It default size it might be too small, so we should probably resize it.

bwindels · 2020-07-31T12:38:26Z

And as well for the top unread button in the timeline please?

mx_TopUnreadMessagesBar_scrollUp::before {
    ...
    mask-image: url('$(res)/img/icon-jump-to-first-unread.svg');
    ...
}

benedikrok · 2020-07-31T13:14:14Z

And as well for the top unread button in the timeline please?
mx_TopUnreadMessagesBar_scrollUp::before {
    ...
    mask-image: url('$(res)/img/icon-jump-to-first-unread.svg');
    ...
}

👍

…rons

jryans · 2020-07-31T19:39:29Z

@benedikrok Today's updates include:

Room list colours should now match Figma
Network dropdown chevron is thinner, moved down slightly to center with adjacent text
Converted jump to bottom button
Converted jump to unread button

See updated screenshots in the top comment to check the results.

benedikrok · 2020-08-03T12:34:02Z

@benedikrok Today's updates include:

Room list colours should now match Figma

Network dropdown chevron is thinner, moved down slightly to center with adjacent text

Converted jump to bottom button

Converted jump to unread button

See updated screenshots in the top comment to check the results.

Looks Good 👍

jryans · 2020-08-03T12:34:33Z

Thanks all! 😄

jryans requested review from a team July 30, 2020 15:51

jryans added 2 commits July 30, 2020 17:40

Tweak size / position in network dropdown

a6d5a2f

Larger chevron for rebrand

7205612

turt2live approved these changes Jul 30, 2020

View reviewed changes

jryans added 7 commits July 31, 2020 17:48

Fix show less icon

8441763

Adjust room list colours to match designs

31180d3

Add comment on network chevron placement

805b8f5

Use thinner chevron for network, adjust placement

1a33104

Merge remote-tracking branch 'origin/develop' into jryans/adjust-chev…

f28bce3

…rons

Convert jump to bottom button

7aaa499

Convert jump to unread button

71af458

turt2live approved these changes Jul 31, 2020

View reviewed changes

jryans merged commit 533b5d8 into develop Aug 3, 2020

Uh oh!

Uh oh!

Conversation

jryans commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Network dropdown

Rebrand modal

Room list

Jump to bottom

Jump to unread

With guides to confirm alignment

Uh oh!

benedikrok commented Jul 30, 2020

Uh oh!

jryans commented Jul 30, 2020

Uh oh!

benedikrok commented Jul 30, 2020

Uh oh!

jryans commented Jul 30, 2020

Uh oh!

benedikrok commented Jul 30, 2020

Uh oh!

jryans commented Jul 30, 2020

Figma

Current app without changes

Light theme

Dark theme

Uh oh!

benedikrok commented Jul 30, 2020

Figma

Current app without changes

Light theme

Dark theme

Uh oh!

turt2live left a comment

Choose a reason for hiding this comment

Uh oh!

turt2live Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

bwindels commented Jul 31, 2020

Uh oh!

benedikrok commented Jul 31, 2020

Uh oh!

bwindels commented Jul 31, 2020

Uh oh!

benedikrok commented Jul 31, 2020

Uh oh!

bwindels commented Jul 31, 2020

Uh oh!

benedikrok commented Jul 31, 2020

Uh oh!

jryans commented Jul 31, 2020

Uh oh!

benedikrok commented Aug 3, 2020

Uh oh!

jryans commented Aug 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jryans commented Jul 30, 2020 •

edited

Loading