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

Make left panel better for new users (mk II) #3804

Merged
merged 9 commits into from May 8, 2017

Conversation

@dbkr
Copy link
Member

dbkr commented May 4, 2017

Implements #3609

Adds RoleButton to reduce duplication as we now have up to 3 copies of the same button.

Requires matrix-org/matrix-react-sdk#859

@dbkr

This comment has been minimized.

Copy link
Member Author

dbkr commented May 4, 2017

(Tests failing because of changes required in react-sdk)

@@ -468,16 +471,18 @@ var RoomSubList = React.createClass({

render: function() {
var connectDropTarget = this.props.connectDropTarget;
var RoomDropTarget = sdk.getComponent('rooms.RoomDropTarget');
var TruncatedList = sdk.getComponent('elements.TruncatedList');

var label = this.props.collapsed ? null : this.props.label;

//console.log("render: " + JSON.stringify(this.state.sortedList));

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 May 4, 2017

Contributor

could remove this

This comment has been minimized.

Copy link
@dbkr

dbkr May 4, 2017

Author Member

done

target = <RoomDropTarget label={ 'Drop here to ' + this.props.verb }/>;
let content;
if (this.state.sortedList.length == 0) {
//content = <RoomDropTarget label={ 'Drop here to ' + this.props.verb }/>;

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 May 4, 2017

Contributor

This implies that it will always be "Drop here to ...". Not sure how useful that would be

This comment has been minimized.

Copy link
@dbkr

dbkr May 4, 2017

Author Member

Hmm, yeah, removed

dbkr added 2 commits May 4, 2017
@lukebarnard1

This comment has been minimized.

Copy link
Contributor

lukebarnard1 commented May 4, 2017

@dbkr I forgot to mention that the padding around the text isn't even:
2017-05-04-175300_224x67_scrot

@lukebarnard1

This comment has been minimized.

Copy link
Contributor

lukebarnard1 commented May 4, 2017

And also, there's no chevron as per the design that points to the Room sublist header

@dbkr

This comment has been minimized.

Copy link
Member Author

dbkr commented May 4, 2017

Yeah, I've left Matthew to do the chevron as it'll require a special SVG to be drawn.

@dbkr

This comment has been minimized.

Copy link
Member Author

dbkr commented May 4, 2017

and I think the padding is even, it's just that it's left-justified - either way I was going to leave matthew to tweak the CSS

@lukebarnard1

This comment has been minimized.

Copy link
Contributor

lukebarnard1 commented May 4, 2017

Fair enough. @ara4n I think the buttons could do with vertical-align: -4px; when you get around to CSS tweaking.

@dbkr

This comment has been minimized.

Copy link
Member Author

dbkr commented May 4, 2017

Oh I see what you mean, yes (I went with -3 as it looked better to me, easy to tweak though)

@lukebarnard1 lukebarnard1 assigned dbkr and unassigned lukebarnard1 May 5, 2017
@dbkr dbkr merged commit e613382 into develop May 8, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@lampholder lampholder added the notready label May 15, 2017
dbkr added a commit that referenced this pull request May 17, 2017
…ewbies_2"

This reverts commit e613382, reversing
changes made to d1db602.
dbkr added a commit that referenced this pull request May 17, 2017
…el_for_newbies_2""

This reverts commit 3cead03.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.