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

Separation of concerns between IComponentHTMLView and IComponentHTMLVisual #1230

Merged
merged 30 commits into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0ddc808
Remove calls to addView() from within some render() implementations
ChumpChief Feb 5, 2020
e890062
Remove some direct usage of IComponentHTMLRender
ChumpChief Feb 5, 2020
61b7c5e
More render->visual
ChumpChief Feb 5, 2020
f17ab5b
Update comment
ChumpChief Feb 5, 2020
d10d563
Push down IComponentHTMLRender
ChumpChief Feb 5, 2020
0476d01
Make remove optional
ChumpChief Feb 6, 2020
2196354
Remove intel viewer's addView, cloning not necessary
ChumpChief Feb 6, 2020
18ccf2d
Start add IComponentHTMLView to IComponentHTMLVisual classes
ChumpChief Feb 6, 2020
e6f58f9
Continue add IComponentHTMLView to IComponentHTMLVisual classes
ChumpChief Feb 6, 2020
dc75ba9
Continue add IComponentHTMLView to IComponentHTMLVisual classes
ChumpChief Feb 6, 2020
6ca3b05
Continue add IComponentHTMLView to IComponentHTMLVisual classes
ChumpChief Feb 6, 2020
4408acc
Continue add IComponentHTMLView to IComponentHTMLVisual classes
ChumpChief Feb 6, 2020
67b3ba7
Add IProvideComponentHTMLView with optional member
ChumpChief Feb 6, 2020
733ff77
Modify places seeking IComponentHTMLVisuals to also accept IComponent…
ChumpChief Feb 6, 2020
0d3891c
More removal of visual
ChumpChief Feb 7, 2020
33f82c7
Start swapping visual to view where appropriate
ChumpChief Feb 7, 2020
8c13ea6
More swapping visual to view where appropriate
ChumpChief Feb 7, 2020
fa48aa1
More swapping visual to view where appropriate
ChumpChief Feb 7, 2020
b49e9c5
More swapping visual to view where appropriate
ChumpChief Feb 7, 2020
8a3f15a
Finish swapping visual to view where appropriate
ChumpChief Feb 7, 2020
ea7f7c7
Comment fixup
ChumpChief Feb 7, 2020
22e1452
Require a view
ChumpChief Feb 7, 2020
5ec5383
Make IComponentHTMLView a mandatory member of IProvideComponentHTMLVi…
ChumpChief Feb 7, 2020
1204e7f
Remove render as member of IComponentHTMLVisual
ChumpChief Feb 7, 2020
5cb3509
For components that were visuals and views, but actually had view sep…
ChumpChief Feb 7, 2020
302c83c
Tweak comments
ChumpChief Feb 7, 2020
5e915bf
Fix rebase error
ChumpChief Feb 7, 2020
b63bc09
Add more comments, fix merge
ChumpChief Feb 11, 2020
6303e6e
Add breaking change notice
ChumpChief Feb 11, 2020
e26ee6e
Tweak breaking change notice
ChumpChief Feb 13, 2020
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
8 changes: 8 additions & 0 deletions BREAKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- [Remove back-compat support for loader <= 0.8](#remove-back-compat-support-for-loader-0.8)
- [New Error types](#New-Error-types)
- [`IComponentContext` - `createSubComponent` removed, `createComponent` signature updated](#`IComponentContext`---`createSubComponent`-removed,-`createComponent`-signature-updated)
- [Changes to the render interfaces](#Changes-to-the-render-interfaces)

## Samples and chaincode have been renamed to examples and components respectively
The directories themselves have been renamed.
Expand Down Expand Up @@ -35,6 +36,13 @@ It does not acccept a package path anymore but just a package name. To pass in p

For creating a component with a specific package path, use `createComponent` or `_createComponentWithProps` in `IHostRuntime`.

## Changes to the render interfaces

The rendering interfaces have undergone several changes:
- `IComponentHTMLRender` has been removed. `IComponentHTMLView` now has a `render()` member, and `IComponentHTMLVisual` does not. If your component renders, it should probably be an `IComponentHTMLView`.
- Since `IComponentHTMLVisual` now only has the member `addView()`, it is mandatory. If your component does not already implement `addView`, it should not be an `IComponentHTMLVisual`.
- On `IComponentHTMLView`, `remove()` is now optional. If your view component needs to perform cleanup when removed from the DOM, do it in `remove()` - otherwise there is no need to implement it.
- `IComponentHTMLView` now extends the new `IProvideComponentHTMLView`, so you can query for whether a component is a view. You must implement the `IComponentHTMLView` member if you implement the interface.

# 0.13 Breaking Changes

Expand Down
12 changes: 8 additions & 4 deletions examples/components/badge/src/Badge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
import { PrimedComponent } from "@microsoft/fluid-aqueduct";
import { IComponentReactViewable } from "@microsoft/fluid-aqueduct-react";
import { SharedCell } from "@microsoft/fluid-cell";
import { IComponentHandle, IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import {
IComponentHandle,
IComponentHTMLView,
} from "@microsoft/fluid-component-core-interfaces";
import { SharedMap } from "@microsoft/fluid-map";
import { SharedObjectSequence } from "@microsoft/fluid-sequence";
// eslint-disable-next-line import/no-internal-modules
Expand All @@ -17,13 +20,14 @@ import { IBadgeType } from "./IBadgeType";
import { BadgeView } from "./BadgeView";
import { IHistory } from "./IHistory";

export class Badge extends PrimedComponent implements IComponentHTMLVisual, IComponentReactViewable {
export class Badge extends PrimedComponent implements
IComponentHTMLView,
IComponentReactViewable {
currentCell: SharedCell;
optionsMap: SharedMap;
historySequence: SharedObjectSequence<IHistory<IBadgeType>>;

public get IComponentHTMLVisual() { return this; }
public get IComponentHTMLRender() { return this; }
public get IComponentHTMLView() { return this; }
public get IComponentReactViewable() { return this; }

private readonly currentId: string = "value";
Expand Down
7 changes: 3 additions & 4 deletions examples/components/chat/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
* Licensed under the MIT License.
*/

import { IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";
import { IContainerContext } from "@microsoft/fluid-container-definitions";
import { renderChat } from "./chat";
// eslint-disable-next-line import/no-internal-modules
import { Runtime } from "./runtime/runtime";

export class ChatRunner implements IComponentHTMLVisual {

public get IComponentHTMLVisual() { return this; }
export class ChatRunner implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

constructor(private readonly runtime: Runtime) {
}
Expand Down
12 changes: 2 additions & 10 deletions examples/components/codemirror/src/codemirror.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class CodemirrorView implements IComponentHTMLView {

private sequenceDeltaCb: any;

public get IComponentHTMLView() { return this; }

constructor(private readonly text: SharedString, private readonly runtime: IComponentRuntime) {
}

Expand Down Expand Up @@ -211,8 +213,6 @@ export class CodeMirrorComponent
private text: SharedString;
private root: ISharedMap;

private defaultView: CodemirrorView;

constructor(
private readonly runtime: IComponentRuntime,
/* Private */ context: IComponentContext,
Expand Down Expand Up @@ -251,14 +251,6 @@ export class CodeMirrorComponent
public addView(scope: IComponent): IComponentHTMLView {
return new CodemirrorView(this.text, this.runtime);
}

public render(elm: HTMLElement, options?: IComponentHTMLOptions): void {
if (!this.defaultView) {
this.defaultView = new CodemirrorView(this.text, this.runtime);
}

this.defaultView.render(elm, options);
}
}

class SmdeFactory implements IComponentFactory {
Expand Down
6 changes: 3 additions & 3 deletions examples/components/draft-js/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
PrimedComponentFactory,
SimpleModuleInstantiationFactory,
} from "@microsoft/fluid-aqueduct";
import { IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";
import { SharedMap } from "@microsoft/fluid-map";
import { SharedString } from "@microsoft/fluid-sequence";

Expand All @@ -22,8 +22,8 @@ import { MemberList } from "./MemberList";
const pkg = require("../package.json");
export const DraftJsName = pkg.name as string;

export class DraftJsExample extends PrimedComponent implements IComponentHTMLVisual {
public get IComponentHTMLVisual() { return this; }
export class DraftJsExample extends PrimedComponent implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

/**
* Do setup work here
Expand Down
10 changes: 4 additions & 6 deletions examples/components/drawer/src/drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
IComponentRouter,
IRequest,
IResponse,
IComponentHTMLOptions,
IComponentHTMLVisual,
IComponent,
IComponentHTMLView,
Expand All @@ -25,7 +24,10 @@ import { initializeIcons } from "@uifabric/icons";
import * as semver from "semver";
import { DrawerView } from "./drawerView";

export class Drawer extends EventEmitter implements IComponentLoadable, IComponentRouter, IComponentHTMLVisual {
export class Drawer extends EventEmitter implements
IComponentLoadable,
IComponentRouter,
IComponentHTMLVisual {
public static async load(runtime: IComponentRuntime, context: IComponentContext) {
const collection = new Drawer(runtime, context);
await collection.initialize();
Expand Down Expand Up @@ -120,10 +122,6 @@ export class Drawer extends EventEmitter implements IComponentLoadable, ICompone

return view;
}

public render(elm: HTMLElement, options?: IComponentHTMLOptions): void {
throw new Error("Just addView please");
}
}

class DrawerFactory implements IComponentFactory {
Expand Down
2 changes: 2 additions & 0 deletions examples/components/drawer/src/drawerView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export class DrawerView implements IComponentHTMLView {
private packages: { pkg: string; name: string; version: string; icon: string }[] = [];
private elm: HTMLElement;

public get IComponentHTMLView() { return this; }

constructor(
private readonly documentsFactory: IDocumentFactory,
private readonly documentsMap: ISharedMap,
Expand Down
10 changes: 3 additions & 7 deletions examples/components/github-comment/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import {
PrimedComponentFactory,
} from "@microsoft/fluid-aqueduct";
import {
IComponentHTMLVisual,
IComponentHTMLView,
IComponentHandle,
IComponentHTMLRender,
} from "@microsoft/fluid-component-core-interfaces";
import {
IComponentContext,
Expand All @@ -35,11 +34,8 @@ const divHTML = require("./styles/github-comment-only.html");
/* eslint-enable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires, import/no-internal-modules,
import/no-unassigned-import */

export class GithubComment
extends TextareaNoReact
implements IComponentHTMLVisual, IComponentHTMLRender {
public get IComponentHTMLVisual() { return this; }
public get IComponentHTMLRender() { return this; }
export class GithubComment extends TextareaNoReact implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

/**
* Extension of the parent class function that also forces the innerHTML of
Expand Down
6 changes: 3 additions & 3 deletions examples/components/image-gallery/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { PrimedComponent, PrimedComponentFactory, SimpleModuleInstantiationFactory } from "@microsoft/fluid-aqueduct";
import { IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";
import * as React from "react";
import * as ReactDOM from "react-dom";
import ImageGallery from "react-image-gallery";
Expand All @@ -14,8 +14,8 @@ import "react-image-gallery/styles/css/image-gallery.css";
import "./Styles.css";
import { ISharedMap } from "@microsoft/fluid-map";

export class ImageGalleryComponent extends PrimedComponent implements IComponentHTMLVisual {
public get IComponentHTMLVisual() { return this; }
export class ImageGalleryComponent extends PrimedComponent implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

imageList = [
{
Expand Down
6 changes: 3 additions & 3 deletions examples/components/musica/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

// Fluid
import { PrimedComponent, PrimedComponentFactory, SimpleModuleInstantiationFactory } from "@microsoft/fluid-aqueduct";
import { IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";

// React
import * as React from "react";
Expand All @@ -19,8 +19,8 @@ import { DAW } from "./daw";
// TODO: Is this right?
const audioContext = new AudioContext();

export class Musica extends PrimedComponent implements IComponentHTMLVisual {
public get IComponentHTMLVisual() { return this; }
export class Musica extends PrimedComponent implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

protected async componentHasInitialized() {
this.player = new Player(audioContext);
Expand Down
10 changes: 4 additions & 6 deletions examples/components/persona/src/persona.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
IComponentRouter,
IRequest,
IResponse,
IComponentHTMLOptions,
IComponentHTMLVisual,
IComponent,
IComponentHTMLView,
Expand All @@ -22,7 +21,10 @@ import { ISharedObjectFactory } from "@microsoft/fluid-shared-object-base";
import { initializeIcons } from "@uifabric/icons";
import { PersonaView } from "./personaView";

export class Persona extends EventEmitter implements IComponentLoadable, IComponentRouter, IComponentHTMLVisual {
export class Persona extends EventEmitter implements
IComponentLoadable,
IComponentRouter,
IComponentHTMLVisual {
public static async load(runtime: IComponentRuntime, context: IComponentContext) {
const collection = new Persona(runtime, context);
await collection.initialize();
Expand Down Expand Up @@ -106,10 +108,6 @@ export class Persona extends EventEmitter implements IComponentLoadable, ICompon

return view;
}

public render(elm: HTMLElement, options?: IComponentHTMLOptions): void {
throw new Error("Just addView please");
}
}

class PersonaFactory implements IComponentFactory {
Expand Down
2 changes: 2 additions & 0 deletions examples/components/persona/src/personaView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export class PersonaReactComponent extends React.Component<IPersonaReactComponen
}

export class PersonaView implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

constructor(private readonly directory: IDirectory, public remove: () => void) {
}

Expand Down
6 changes: 3 additions & 3 deletions examples/components/pollster/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { PrimedComponent, PrimedComponentFactory, SimpleModuleInstantiationFactory } from "@microsoft/fluid-aqueduct";
import { IComponentHTMLVisual, IComponentHandle } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView, IComponentHandle } from "@microsoft/fluid-component-core-interfaces";
import { ISharedMap, SharedMap } from "@microsoft/fluid-map";
import * as React from "react";
import * as ReactDOM from "react-dom";
Expand All @@ -16,8 +16,8 @@ import { Poll } from "./view/Poll";
const pkg = require("../package.json");
const chaincodeName = pkg.name;

export class Pollster extends PrimedComponent implements IComponentHTMLVisual {
public get IComponentHTMLVisual() { return this; }
export class Pollster extends PrimedComponent implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

protected async componentInitializingFirstTime() {
this.root.set(pollOptionsMapKey, SharedMap.create(this.runtime).handle);
Expand Down
25 changes: 17 additions & 8 deletions examples/components/prosemirror/src/componentView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
import { Node } from "prosemirror-model";
import { EditorView, NodeView } from "prosemirror-view";
import { ILoader } from "@microsoft/fluid-container-definitions";
import { IComponent, IComponentHTMLRender, IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";
import { IComponent, IComponentHTMLView, IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";

export class ComponentView implements NodeView {
public dom: HTMLElement;
public innerView;

private renderable: IComponentHTMLView | IComponentHTMLRender;
private visual: IComponentHTMLView | IComponentHTMLVisual;

constructor(
public node: Node,
Expand Down Expand Up @@ -64,7 +64,7 @@ export class ComponentView implements NodeView {
}

const component = result.value as IComponent;
if (!component.IComponentHTMLVisual) {
if (!component.IComponentHTMLView && !component.IComponentHTMLVisual) {
return Promise.reject<IComponent>();
}

Expand All @@ -77,13 +77,22 @@ export class ComponentView implements NodeView {
this.dom.innerHTML = "";

// Remove the previous view
if (this.renderable && "remove" in this.renderable) {
this.renderable.remove();
if (this.visual && "remove" in this.visual) {
this.visual.remove();
}

const visual = component.IComponentHTMLVisual;
this.renderable = visual.addView ? visual.addView() : visual;
this.renderable.render(this.dom);
// First try to get it as a view
this.visual = component.IComponentHTMLView;
if (!this.visual) {
// Otherwise get the visual, which is a view factory
const visual = component.IComponentHTMLVisual;
if (visual) {
this.visual = visual.addView();
}
}
if (this.visual) {
this.visual.render(this.dom);
}
},
(error) => {
// Fall back to URL if can't load
Expand Down
11 changes: 2 additions & 9 deletions examples/components/prosemirror/src/prosemirror.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class ProseMirrorView implements IComponentHTMLView {
private textArea: HTMLDivElement;
private readonly collabManager: FluidCollabManager;

public get IComponentHTMLView() { return this; }

public constructor(
private readonly text: SharedString,
private readonly runtime: IComponentRuntime,
Expand Down Expand Up @@ -118,7 +120,6 @@ export class ProseMirror extends EventEmitter
public text: SharedString;
private root: ISharedMap;
private collabManager: FluidCollabManager;
private defaultView: ProseMirrorView;

constructor(
private readonly runtime: IComponentRuntime,
Expand Down Expand Up @@ -163,14 +164,6 @@ export class ProseMirror extends EventEmitter
public addView(): IComponentHTMLView {
return new ProseMirrorView(this.text, this.runtime);
}

public render(elm: HTMLElement, options?: IComponentHTMLOptions): void {
if (!this.defaultView) {
this.defaultView = new ProseMirrorView(this.text, this.runtime);
}

this.defaultView.render(elm, options);
}
}

class ProseMirrorFactory implements IComponentFactory {
Expand Down
6 changes: 3 additions & 3 deletions examples/components/simple-component-embed/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
*/

import { PrimedComponent, PrimedComponentFactory, SimpleModuleInstantiationFactory } from "@microsoft/fluid-aqueduct";
import { IComponentHTMLVisual } from "@microsoft/fluid-component-core-interfaces";
import { IComponentHTMLView } from "@microsoft/fluid-component-core-interfaces";
import { ClickerInstantiationFactory, Clicker } from "@fluid-example/clicker";

export class SimpleComponentEmbed extends PrimedComponent implements IComponentHTMLVisual {
public get IComponentHTMLVisual() { return this; }
export class SimpleComponentEmbed extends PrimedComponent implements IComponentHTMLView {
public get IComponentHTMLView() { return this; }

private clicker: Clicker;

Expand Down
Loading