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

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented May 5, 2021

@t3chguy t3chguy requested review from a team and niquewoodhouse May 5, 2021 16:26
@niquewoodhouse
Copy link
Contributor

@t3chguy looks great, I just have 4 notes:

  • Position of settings cog
    • Not a big deal but finding it's unbalancing things because it's moving the invite button depending on if I'm an admin of the space or not. I think next to the space name makes more sense for the current layout. Lets wait for more feedback from beta
  • Disabled state of management buttons
    • The hollow opaque style feels like it might not be very accessible. Maybe this is a global issue across Element(?) and not something to fix here. If it is global, I can take that as a design issue, let me know.
    • Sizing of buttons
      • Now I see the buttons more often I'm finding the different sizes of the buttons a bit awkward. I think it's caused by different paddings on the buttons causing different heights making the View button bigger. Maybe it's not relevant to this issue but consistent sizing might be nice. Again, if a global thing, can pick up seperately.
        image
  • View room interaction
    • When I tap the view button, the room becomes selected and I'm redirected to the room. Just found it jarring but probably fine for a beta.
@t3chguy
Copy link
Member Author

t3chguy commented May 6, 2021

If it is global, I can take that as a design issue, let me know.

It is indeed global, its a button kind=primary_outline and kind=danger_outline but disabled

will fix the button size issue and the untrapped behaviour of the join button click


image

The Join button has a touch more horizontal padding as its less space-constrained, can tweak if you want

Copy link
Contributor

@niquewoodhouse niquewoodhouse left a comment

Choose a reason for hiding this comment

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

All looks resolved, it's just this 1 button is 31px for some reason?

It would just be great if possible if it was equal to the others.

image

@t3chguy
Copy link
Member Author

t3chguy commented May 7, 2021

Its 32px here :L

image

I'll give it an explicit rem line-height which should make it more consistent

@t3chguy
Copy link
Member Author

t3chguy commented May 7, 2021

Fixed it for that specific button but a more general solution would be good in the future - opened element-hq/element-web#17197 to track it

@t3chguy t3chguy requested a review from niquewoodhouse May 7, 2021 10:41
@t3chguy t3chguy merged commit 6338ced into develop May 7, 2021
@t3chguy t3chguy deleted the t3chguy/fix/17176 branch May 7, 2021 10:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants