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

Redesign: resizeable/collapsible sections #2210

Merged
merged 28 commits into from Oct 19, 2018

Conversation

bwindels
Copy link
Contributor

includes using flexbox in the room tile to avoid hardcoded width
as the resize handle is a sibling of the mxLeftPanel_container, that
class is the one that has to collapse if we don't want to complicate
the logic. So change style rules to check
.mxLeftPanel_container.collapsed, and make left panel not break
out of the container when it gets narrow by hiding the overflow
@bwindels
Copy link
Contributor Author

As I've already made these changes available on experimental to be able to show the WIP, I can't make the base that branch. For now merging into bwindels/redesign and merge that into experimental afterwards. In the feature, stuff will only go on experimental once approved though.

@bwindels
Copy link
Contributor Author

The e2e tests are broken because the "create room button" is missing for now in the redesign ftr.

@bwindels bwindels removed the notready label Oct 17, 2018
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

otherwise lgtm!

@@ -155,7 +155,7 @@ class Tinter {

tint(primaryColor, secondaryColor, tertiaryColor) {
return;

// eslint-disable-next-line no-unreachable
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't eslint have a valid point here? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does :) The return got added as part of redesign experiments. I'd rather wait a bit to throw the whole Tinter out if that is OK.

@@ -507,14 +558,16 @@ const LoggedInView = React.createClass({
<div className='mx_MatrixChat_wrapper' aria-hidden={this.props.hideToSRUsers} onMouseDown={this._onMouseDown} onMouseUp={this._onMouseUp}>
{ topBar }
<DragDropContext onDragEnd={this._onDragEnd}>
<div className={bodyClasses}>
<div ref={(div) => this.resizeContainer = div} className={bodyClasses}>
Copy link
Member

Choose a reason for hiding this comment

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

LoggedInView probably is a class where we want to avoid passing anonymous functions here if possible as we don't want it to re-render unnecessarily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so basically do ref={this.resizeContainer}? Curious how this causes a re-render.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, because each new instance of the function !== the previous, causing the subcomponent to rerender.

src/resizer/resizer.js Outdated Show resolved Hide resolved
src/resizer/resizer.js Show resolved Hide resolved
@bwindels bwindels merged commit 9330281 into bwindels/redesign Oct 19, 2018
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.

None yet

3 participants