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

Vertical two-up #100

Merged
merged 4 commits into from
Jul 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
69 changes: 50 additions & 19 deletions src/components/Output/custom-els/TwoUp/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import * as styles from './styles.css';
import { PointerTracker, Pointer } from '../../../../lib/PointerTracker';

const legacyClipCompat = 'legacy-clip-compat';
const legacyClipCompatAttr = 'legacy-clip-compat';
const verticalAttr = 'vertical';
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be like the disabled attribute, where by presence indicates the state? I was wondering why this wasn't just an orientation attribute with a type that has a default of horizontal, but the option of 'vertical'


/**
* A split view that the user can adjust. The first child becomes
* the left-hand side, and the second child becomes the right-hand side.
*/
export default class TwoUp extends HTMLElement {
static get observedAttributes () { return [legacyClipCompat]; }
static get observedAttributes() { return [verticalAttr]; }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had legacyClipCompat here previously, but it was a mistake.


private readonly _handle = document.createElement('div');
/**
Expand Down Expand Up @@ -52,39 +53,64 @@ export default class TwoUp extends HTMLElement {
});
}

connectedCallback () {
connectedCallback() {
this._childrenChange();
if (!this._everConnected) {
// Set the initial position of the handle.
requestAnimationFrame(() => {
const bounds = this.getBoundingClientRect();
this._position = bounds.width / 2;
this._setPosition();
});
this._resetPosition();
this._everConnected = true;
}
}

attributeChangedCallback(name: string) {
if (name === verticalAttr) {
this._resetPosition();
}
}

private _resetPosition() {
// Set the initial position of the handle.
requestAnimationFrame(() => {
const bounds = this.getBoundingClientRect();
this._position = (this.vertical ? bounds.height : bounds.width) / 2;
this._setPosition();
});
}

/**
* If true, this element works in browsers that don't support clip-path (Edge).
* However, this means you'll have to set the height of this element manually.
*/
get noClipPathCompat () {
return this.hasAttribute(legacyClipCompat);
get legacyClipCompat() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I renamed the attr but forgot to rename the prop.

return this.hasAttribute(legacyClipCompatAttr);
}

set noClipPathCompat (val: boolean) {
set legacyClipCompat(val: boolean) {
if (val) {
this.setAttribute(legacyClipCompat, '');
this.setAttribute(legacyClipCompatAttr, '');
} else {
this.removeAttribute(legacyClipCompat);
this.removeAttribute(legacyClipCompatAttr);
}
}

/**
* Split vertically rather than horizontally.
*/
get vertical() {
return this.hasAttribute(verticalAttr);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's repetition here, but I'm not sure if there's a typescript-friendly way to solve it.

I'd usually transform the attr names to property names then generate the getters/setters automatically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could write a decorator for tying attrs with props. Not sure if it’s worth it (yet).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we feel we need it, we should talk to the polymer folks. Pretty sure they'll have thought about this before.


set vertical(val: boolean) {
if (val) {
this.setAttribute(verticalAttr, '');
} else {
this.removeAttribute(verticalAttr);
}
}

/**
* Called when element's child list changes
*/
private _childrenChange () {
private _childrenChange() {
// Ensure the handle is the last child.
// The CSS depends on this.
if (this.lastElementChild !== this._handle) {
Expand All @@ -95,15 +121,20 @@ export default class TwoUp extends HTMLElement {
/**
* Called when a pointer moves.
*/
private _pointerChange (startPoint: Pointer, currentPoint: Pointer) {
private _pointerChange(startPoint: Pointer, currentPoint: Pointer) {
const pointAxis = this.vertical ? 'clientY' : 'clientX';
const dimensionAxis = this.vertical ? 'height' : 'width';
const bounds = this.getBoundingClientRect();
this._position = this._positionOnPointerStart + (currentPoint.clientX - startPoint.clientX);

this._position = this._positionOnPointerStart +
(currentPoint[pointAxis] - startPoint[pointAxis]);

// Clamp position to element bounds.
this._position = Math.max(0, Math.min(this._position, bounds.width));
this._position = Math.max(0, Math.min(this._position, bounds[dimensionAxis]));
this._setPosition();
}

private _setPosition () {
private _setPosition() {
this.style.setProperty('--split-point', `${this._position}px`);
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/components/Output/custom-els/TwoUp/missing-types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ interface Window {
Touch: typeof Touch;
}

interface TwoUpAttributes extends JSX.HTMLAttributes {
'vertical'?: boolean;
'legacy-clip-compat'?: boolean;
}

declare namespace JSX {
interface IntrinsicElements {
'two-up': HTMLAttributes;
'two-up': TwoUpAttributes;
}
}
29 changes: 29 additions & 0 deletions src/components/Output/custom-els/TwoUp/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ two-up[legacy-clip-compat] > :not(.twoUpHandle) {
border-radius: 20px;
}

two-up[vertical] .twoUpHandle {
width: auto;
height: 10px;
transform: translateY(var(--split-point)) translateY(-50%);
}

two-up[vertical] .twoUpHandle::after {
width: 40px;
height: 80px;
}

two-up > :nth-child(1):not(.twoUpHandle) {
-webkit-clip-path: inset(0 calc(100% - var(--split-point)) 0 0);
clip-path: inset(0 calc(100% - var(--split-point)) 0 0);
Expand All @@ -46,6 +57,16 @@ two-up > :nth-child(2):not(.twoUpHandle) {
clip-path: inset(0 0 0 var(--split-point));
}

two-up[vertical] > :nth-child(1):not(.twoUpHandle) {
-webkit-clip-path: inset(0 0 calc(100% - var(--split-point)) 0);
clip-path: inset(0 0 calc(100% - var(--split-point)) 0);
}

two-up[vertical] > :nth-child(2):not(.twoUpHandle) {
-webkit-clip-path: inset(var(--split-point) 0 0 0);
clip-path: inset(var(--split-point) 0 0 0);
}

/*
Even in legacy-clip-compat, prefer clip-path if it's supported.
It performs way better in Safari.
Expand All @@ -58,4 +79,12 @@ two-up > :nth-child(2):not(.twoUpHandle) {
two-up[legacy-clip-compat] > :nth-child(2):not(.twoUpHandle) {
clip: rect(auto auto auto var(--split-point));
}

two-up[vertical][legacy-clip-compat] > :nth-child(1):not(.twoUpHandle) {
clip: rect(auto auto var(--split-point) auto);
}

two-up[vertical][legacy-clip-compat] > :nth-child(2):not(.twoUpHandle) {
clip: rect(var(--split-point) auto auto auto);
}
}
36 changes: 27 additions & 9 deletions src/components/Output/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,31 @@ import * as style from './style.scss';
import { bind, drawBitmapToCanvas, linkRef } from '../../lib/util';
import { twoUpHandle } from './custom-els/TwoUp/styles.css';

type Props = {
leftImg: ImageBitmap,
rightImg: ImageBitmap,
};
interface Props {
leftImg: ImageBitmap;
rightImg: ImageBitmap;
}

type State = {};
interface State {
verticalTwoUp: boolean;
}

export default class Output extends Component<Props, State> {
state: State = {};
widthQuery = window.matchMedia('(min-width: 500px)');
state: State = {
verticalTwoUp: !this.widthQuery.matches,
};
canvasLeft?: HTMLCanvasElement;
canvasRight?: HTMLCanvasElement;
pinchZoomLeft?: PinchZoom;
pinchZoomRight?: PinchZoom;
retargetedEvents = new WeakSet<Event>();

constructor() {
super();
this.widthQuery.addListener(this.onMobileWidthChange);
}

componentDidMount() {
if (this.canvasLeft) {
drawBitmapToCanvas(this.canvasLeft, this.props.leftImg);
Expand All @@ -39,8 +49,15 @@ export default class Output extends Component<Props, State> {
}
}

shouldComponentUpdate(nextProps: Props) {
return this.props.leftImg !== nextProps.leftImg || this.props.rightImg !== nextProps.rightImg;
shouldComponentUpdate(nextProps: Props, nextState: State) {
return this.props.leftImg !== nextProps.leftImg ||
this.props.rightImg !== nextProps.rightImg ||
this.state.verticalTwoUp !== nextState.verticalTwoUp;
}

@bind
onMobileWidthChange() {
this.setState({ verticalTwoUp: !this.widthQuery.matches });
}

@bind
Expand Down Expand Up @@ -80,10 +97,11 @@ export default class Output extends Component<Props, State> {
this.pinchZoomLeft.dispatchEvent(clonedEvent);
}

render({ leftImg, rightImg }: Props, { }: State) {
render({ leftImg, rightImg }: Props, { verticalTwoUp }: State) {
return (
<div class={style.output}>
<two-up
vertical={verticalTwoUp}
// Event redirecting. See onRetargetableEvent.
onTouchStartCapture={this.onRetargetableEvent}
onTouchEndCapture={this.onRetargetableEvent}
Expand Down