Skip to content

Fix and simplify resize handling using ResizeObserver #3411

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 21, 2019
Merged

Fix and simplify resize handling using ResizeObserver #3411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08e83f3
Fix and simplify resize handling using ResizeObserver
igorsdv Jan 12, 2019
b26f3f4
Adding setTimeout to resizeObserver
juancampa Jan 21, 2019
d78e9ef
Resizing terms in one step and using transform for animation
juancampa Jan 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions lib/components/term-group.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class TermGroup_ extends React.PureComponent {
super(props, context);
this.bound = new WeakMap();
this.termRefs = {};
this.sizeChanged = false;
this.onTermRef = this.onTermRef.bind(this);
}

Expand Down Expand Up @@ -102,16 +101,6 @@ class TermGroup_ extends React.PureComponent {
return <Term ref_={this.onTermRef} key={uid} {...props} />;
}

componentWillReceiveProps(nextProps) {
if (this.props.termGroup.sizes != nextProps.termGroup.sizes || nextProps.sizeChanged) {
this.term && this.term.fitResize();
// Indicate to children that their size has changed even if their ratio hasn't
this.sizeChanged = true;
} else {
this.sizeChanged = false;
}
}

render() {
const {childGroups, termGroup} = this.props;
if (termGroup.sessionUid) {
Expand All @@ -122,10 +111,7 @@ class TermGroup_ extends React.PureComponent {
const props = getTermGroupProps(
child.uid,
this.props.parentProps,
Object.assign({}, this.props, {
termGroup: child,
sizeChanged: this.sizeChanged
})
Object.assign({}, this.props, {termGroup: child})
);

return <DecoratedTermGroup key={child.uid} {...props} />;
Expand Down
48 changes: 16 additions & 32 deletions lib/components/term.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global Blob,URL,requestAnimationFrame */
/* global Blob,URL,requestAnimationFrame,ResizeObserver */
import React from 'react';
import {Terminal} from 'xterm';
import * as fit from 'xterm/lib/addons/fit/fit';
Expand Down Expand Up @@ -78,8 +78,6 @@ export default class Term extends React.PureComponent {
this.termRef = null;
this.termWrapperRef = null;
this.termRect = null;
this.onOpen = this.onOpen.bind(this);
this.onWindowResize = this.onWindowResize.bind(this);
this.onWindowPaste = this.onWindowPaste.bind(this);
this.onTermRef = this.onTermRef.bind(this);
this.onTermWrapperRef = this.onTermWrapperRef.bind(this);
Expand All @@ -106,8 +104,6 @@ export default class Term extends React.PureComponent {
this.term.focus();
}

this.onOpen(this.termOptions);

if (props.onTitle) {
this.disposableListeners.push(this.term.addDisposableListener('title', props.onTitle));
}
Expand Down Expand Up @@ -144,26 +140,13 @@ export default class Term extends React.PureComponent {
);
}

window.addEventListener('resize', this.onWindowResize, {
passive: true
});

window.addEventListener('paste', this.onWindowPaste, {
capture: true
});

terms[this.props.uid] = this;
}

onOpen() {
// we need to delay one frame so that styles
// get applied and we can make an accurate measurement
// of the container width and height
requestAnimationFrame(() => {
this.fitResize();
});
}

getTermDocument() {
// eslint-disable-next-line no-console
console.warn(
Expand All @@ -176,10 +159,6 @@ export default class Term extends React.PureComponent {
return document;
}

onWindowResize() {
this.fitResize();
}

// intercepting paste event for any necessary processing of
// clipboard data, if result is falsy, paste event continues
onWindowPaste(e) {
Expand Down Expand Up @@ -276,12 +255,6 @@ export default class Term extends React.PureComponent {

this.termOptions = nextTermOptions;

if (!this.props.isTermActive && nextProps.isTermActive) {
requestAnimationFrame(() => {
this.fitResize();
});
}

if (
this.props.fontSize !== nextProps.fontSize ||
this.props.fontFamily !== nextProps.fontFamily ||
Expand All @@ -299,6 +272,21 @@ export default class Term extends React.PureComponent {

onTermWrapperRef(component) {
this.termWrapperRef = component;

if (component) {
this.resizeObserver = new ResizeObserver(() => {
if (this.resizeTimeout) {
return;
}
this.resizeTimeout = setTimeout(() => {
delete this.resizeTimeout;
this.fitResize();
}, 0);
});
this.resizeObserver.observe(component);
} else {
this.resizeObserver.disconnect();
}
}

onTermRef(component) {
Expand All @@ -316,10 +304,6 @@ export default class Term extends React.PureComponent {
this.disposableListeners.forEach(handler => handler.dispose());
this.disposableListeners = [];

window.removeEventListener('resize', this.onWindowResize, {
passive: true
});

window.removeEventListener('paste', this.onWindowPaste, {
capture: true
});
Expand Down
27 changes: 25 additions & 2 deletions lib/components/terms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export default class Terms extends React.Component {
render() {
const shift = !isMac && this.props.termGroups.length > 1;
return (
<div className={`terms_terms ${shift ? 'terms_termsShifted' : ''}`}>
<div className={`terms_terms ${shift ? 'terms_termsShifted' : 'terms_termsNotShifted'}`}>
{this.props.customChildrenBefore}
{this.props.termGroups.map(termGroup => {
const {uid} = termGroup;
Expand Down Expand Up @@ -144,11 +144,34 @@ export default class Terms extends React.Component {
left: 0;
bottom: 0;
color: #fff;
transition: ${isMac ? 'none' : 'margin-top 0.3s ease'};
}

.terms_termsShifted {
margin-top: 68px;
animation: shift-down 0.2s ease-out;
Copy link
Contributor

Choose a reason for hiding this comment

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

I made the shift-down a little faster (and eased out) because it overlaps a bit with the tabs

}

.terms_termsNotShifted {
margin-top: 34px;
animation: shift-up 0.3s ease;
}

@keyframes shift-down {
0% {
transform: translateY(-34px);
}
100% {
transform: translateY(0px);
}
}

@keyframes shift-up {
0% {
transform: translateY(34px);
}
100% {
transform: translateY(0px);
}
}

.terms_termGroup {
Expand Down