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

Wmwragg/mention state menu #369

Merged
merged 40 commits into from
Aug 3, 2016
Merged

Wmwragg/mention state menu #369

merged 40 commits into from
Aug 3, 2016

Conversation

wmwragg
Copy link
Contributor

@wmwragg wmwragg commented Jul 29, 2016

A companion branch vector-im/vector-web:wmwragg/mention-state-menu also needs to be merged.

Signed-off-by: William Wragg wm.wragg@gmail.com

…ention-state-menu

# resolved Conflicts:
#	src/components/views/rooms/RoomTile.js
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@wmwragg
Copy link
Contributor Author

wmwragg commented Jul 29, 2016

I'll do a merge up from develop and resolve the conflicts, then add another commit

@ara4n
Copy link
Member

ara4n commented Jul 29, 2016

woo, this is very pretty - thanks :)

Transplanting comments for the record:

  • specifically, wondering about the gap in the hitmask between the roomtile name and the hoverover context menu <-- happy to wait on this until we have signed-off design on the room tile.
  • and if there are any hoverover states for the context menu itself
  • final feedback is that the the gap between the context menu and the LeftPanel is wrong (there should be a small gap in Trevor's design, but it's inset by a few pixels now)

thanks! will wait until you're happy to merge this obv.

@wmwragg
Copy link
Contributor Author

wmwragg commented Jul 29, 2016

Did I miss some hoverover states for the context menu? I didn't think there were any, I've only implemented the click event to change the state

@ara4n
Copy link
Member

ara4n commented Jul 29, 2016

This was more a question for @antikewl - whether the design intends there to be hoverover states. So far the designs have been missing hoverovers, but I'm not sure that this is intentional.

@wmwragg
Copy link
Contributor Author

wmwragg commented Aug 1, 2016

@ara4n I looked at what was involved in doing the CSS re-write over the weekend, and it ended up being rather large, and touched a lot of other modules and their CSS, which touched other modules and their CSS etc... So for this ticket I have left it, as this is focussed on the context menu for notifications. When I do the ticket for the LHS panel restyling, I'll look at doing the CSS refactor then.

So the code is pretty much final, I'm only waiting on Trevor for the designs for collapsed mute state, and long room names, and then I'll mark this as done.

@ara4n
Copy link
Member

ara4n commented Aug 1, 2016

@wmwragg cool. the only thing stopping me from merging this is the question marks hovering over the RoomTile tooltip design itself (and the fact that we haven't wired up the various mute states yet).

I'm a bit scared at the number of open PRs we're starting to accumulate; what are your thoughts on merging this, including the WIP design? Or should we wait?

@wmwragg
Copy link
Contributor Author

wmwragg commented Aug 1, 2016

@ara4n I would prefer to wait for the final designs for the long room name and collapsed mute state before merging this, as I would like a PR to close a ticket, rather than multiple PRs to close a ticket, as that will start to get unmanageable. As far as I can tell, there are only 2 PRs waiting to be merged for the design work ribot is doing (well 4 as there are the matching 2 on the vector-web repo as well, but I view those as the same PR). I'm hoping Trevor (@antikewl) will have me unblocked today.

@wmwragg
Copy link
Contributor Author

wmwragg commented Aug 1, 2016

@ara4n For the RoomTile tooltip design, are we talking about the collapsed tooltip, or the current uncollapsed long hover tooltip? As we can't style the latter it's browser specific and we have no styling control over it.

@ara4n
Copy link
Member

ara4n commented Aug 1, 2016

uh, i have no idea why i said tooltip there - i think i meant 'question marks hovering over the RoomTile design itself'. sorry; was written pre-coffee :(

In terms pending PRs: i think we have 2 for the mention state (one for RoomTile; one for the menu) and one for the buttons. I'll sort the buttons one now, and then happy to consider the two RoomTile ones as effectively same issue.

@wmwragg
Copy link
Contributor Author

wmwragg commented Aug 1, 2016

I think there is only one for the mention state, the one for the RoomTile is just a branch, I merged it into this branch, so it's in this PR. I needed the stuff from that branch as I did further work waiting for the long name design. When this PR is merged it will close issues #1841 and #1746 from the vector-web repo. As far as I am aware these are the outstanding paired PRs:

Note: I've been making the primary PR the one on the vector-web repo, as that repo holds the issues.

@wmwragg
Copy link
Contributor Author

wmwragg commented Aug 2, 2016

@ara4n New long name design. I've also tweaked the gutters so they correctly align. You'll need to pull the vector-web repo as well.

onAction: function(payload) {
switch (payload.action) {
case 'notification_change':
// Is the notificaion about this room
Copy link
Member

Choose a reason for hiding this comment

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

typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well spotted. Fixed

@ara4n
Copy link
Member

ara4n commented Aug 3, 2016

LGTM - thanks for persevering on the epic; i think the end result was worth it :)

@ara4n ara4n merged commit f95a11a into develop Aug 3, 2016
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.

3 participants