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.

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

Room List filtering visual tweaks #5123

Merged
merged 8 commits into from
Aug 18, 2020
Merged

Room List filtering visual tweaks #5123

merged 8 commits into from
Aug 18, 2020

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Aug 17, 2020

@niquewoodhouse
Copy link
Contributor

niquewoodhouse commented Aug 17, 2020

Overall, looks good. Some minor design comments:

Explore icon and text seem to be different link elements.

Really small thing but it makes them a bit harder to use. If difficult, don't think it's super important at all.

explore-link-1

Font-weights

Trying to minimise the amount of font-weights used arbitrarily, unfortunately Figma is behind in this regard. Sorry about that
.mx_RoomList_explorePrompt div:first-child: font-weight 500 600,
.mx_LeftPanel .mx_LeftPanel_roomListContainer .mx_LeftPanel_roomListFilterCount: : font-weight 500 600.
Is there a possibility we have a css typography file that includes a class of semi-bold and that can be referred to in design/development?

Margins

.mx_LeftPanel .mx_LeftPanel_roomListContainer .mx_LeftPanel_roomListFilterCount margin-top 14px works better than 16px in practice. I'll make a note to update figma later this week.

Not necessarily relevant to this issue but noting anyway:

  • Feels slightly odd that on search, sublists aren't fully expanded. I think there's already an issue for this somewhere already.
  • It feels awkward, to me, that the search is above breadcrumbs and especially as search does nothing to breadcrumbs but I understand that's not part of this problem.

@t3chguy
Copy link
Member Author

t3chguy commented Aug 18, 2020

It feels awkward, to me, that the search is above breadcrumbs and especially as search does nothing to breadcrumbs but I understand that's not part of this problem.

Agreed, that was done during rebranding.

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.

lgtm

Base automatically changed from t3chguy/room-list/14617.2 to develop August 18, 2020 11:44
@t3chguy t3chguy requested a review from a team August 18, 2020 11:44
@t3chguy t3chguy marked this pull request as ready for review August 18, 2020 11:44
@t3chguy t3chguy merged commit 24a390f into develop Aug 18, 2020
@t3chguy t3chguy deleted the t3chguy/room-list/14466 branch August 18, 2020 16:22
@luixxiul luixxiul mentioned this pull request May 1, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New room list: Easy to forget you're filtering
3 participants