Taggle Fusion Canvas Texture Renderer by domdir · Pull Request #561 · Caleydo/lineupjs

domdir · 2018-09-07T09:01:15Z

Summary

Introduces a new overview renderer.

In order to display big datasets the columns are rendered onto a canvas. With this approach it is possible to give an overview over very large datasets.

The user is also able to define an expand area. This can be done bei holding down the ALT-key and draging a selection area:

The expand selection is displayed in a seperate column:

The user can expand the selected area to see the row details:

…and Caleydo/lineup_app#4

Caleydo/lineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

…app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

…ineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

…app#3 Caleydo/lineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

…neup_app#22 Caleydo/lineup_app#23

# Conflicts: # src/model/index.ts

thinkh · 2018-10-29T10:32:39Z

I get a runtime error when switching to the overview mode:

npm start
Open http://localhost:8080/builder4.html (or any other demo)
Activate overview checkbox

Uncaught TypeError: Cannot set property 'innerHTML' of null
    at Taggle.setViolation (Taggle.ts:127)
    at Object.violationChanged (Taggle.ts:26)
    at TaggleRenderer.dynamicHeight (TaggleRenderer.ts:92)
    at Object.dynamicHeight (TaggleRenderer.ts:49)
    at EngineRenderer.heightsFor (EngineRenderer.ts:349)
    at EngineRenderer.render (EngineRenderer.ts:389)
    at rankings.forEach (EngineRenderer.ts:378)
    at Array.forEach (<anonymous>)
    at EngineRenderer.update (EngineRenderer.ts:377)
    at TaggleRenderer.update (TaggleRenderer.ts:163)

It seems that the <div class="${cssClass('rule-violation')}"></div> from line 92 is not set because of the surrounding if statement.

thinkh · 2018-10-29T10:35:19Z

src/provider/ADataProvider.ts

    this.fire(ADataProvider.EVENT_SELECTION_CHANGED, [], false);
  }

+  clearDetail() {


Please add function comment

thinkh · 2018-10-29T10:35:23Z

src/provider/ADataProvider.ts

    return Array.from(this.selection);
  }

+  getDetail() {


Please add function comment

thinkh · 2018-10-29T10:35:27Z

src/provider/ADataProvider.ts

    return this.view(this.getSelection());
  }

+  detailRows(): Promise<any[]> | any[] {


Please add function comment

thinkh · 2018-10-29T10:35:44Z

src/provider/ADataProvider.ts

    this.fire(ADataProvider.EVENT_SELECTION_CHANGED, this.getSelection());
  }

+  removeDetailAll(indices: number[]) {


Please add function comment

thinkh · 2018-10-29T10:36:08Z

src/provider/ADataProvider.ts

    return true;
  }

+  toggleDetail(index: number, additional = false) {


Please add function comment

thinkh · 2018-10-29T10:36:13Z

src/provider/ADataProvider.ts

    this.fire(ADataProvider.EVENT_SELECTION_CHANGED, this.getSelection());
  }

+  removeDetail(index: number) {


Please add function comment

thinkh · 2018-10-29T10:36:19Z

src/provider/ADataProvider.ts

    this.selectAll(indices);
  }

+  setDetail(indices: number[]) {


Please add function comment

thinkh · 2018-10-29T10:36:43Z

src/provider/ADataProvider.ts

    this.fire(ADataProvider.EVENT_SELECTION_CHANGED, this.getSelection());
  }

+  addDetailAll(indices: number[]) {


Please add function comment

thinkh · 2018-10-29T10:37:08Z

src/provider/ADataProvider.ts

    this.fire(ADataProvider.EVENT_SELECTION_CHANGED, this.getSelection());
  }

+  addDetail(index: number) {


Please add function comment

thinkh · 2018-10-29T10:37:14Z

src/provider/ADataProvider.ts

    return this.selection.has(index);
  }

+  isDetail(index: number) {


Please add function comment

thinkh · 2018-10-29T10:38:01Z

src/provider/ADataProvider.ts

   */
  private readonly selection = new OrderedSet<number>();

+  private readonly detail = new OrderedSet<number>();


Add property comment

thinkh · 2018-10-29T10:40:05Z

src/styles/renderer/_index.scss

-@import './threshold';
-@import './upset';
-@import './verticalbar';
+.#{$lu_css_prefix} {


Why is this necessary?

thinkh · 2018-10-29T10:45:20Z

src/ui/EngineRanking.ts

  };

-  constructor(public readonly ranking: Ranking, header: HTMLElement, body: HTMLElement, tableId: string, style: GridStyleManager, private readonly ctx: IEngineRankingContext, roptions: Partial<IEngineRankingOptions> = {}) {
+  constructor(public readonly ranking: Ranking, header: HTMLElement, body: HTMLElement, tableId: string, style: GridStyleManager, private readonly ctx: IEngineRankingContext, roptions: Partial<IEngineRankingOptions> = {}, readonly  noEvents: boolean = false) {


Invert semantic of noEvents = false for easier reading. Because in line 188 the counter part is used if(!noEvents). Rename to addEventListener = true.

thinkh · 2018-10-29T10:46:38Z

src/ui/EngineRanking.ts

-      if (newValue) {
-        // become visible
-        const index = ranking.children.indexOf(col);
+    if (!noEvents) {


Please add a comment why and when no event listner is added? Otherwise nobody understands why this flag exists.

thinkh · 2018-10-29T11:55:19Z

src/ui/EngineRenderer.ts

+    d3.select(this.node).select('main').style('display', 'none');
+  }
+
+  s2d() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:55:24Z

src/ui/EngineRenderer.ts

+    this.textureRenderer.s2d();
+  }
+
+  d2s() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:56:04Z

src/ui/CanvasTextureRenderer.ts

+    this.node.style.display = 'none';
+  }
+
+  s2d() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:56:09Z

src/ui/CanvasTextureRenderer.ts

+    this.engineRenderer.ctx.provider.setDetail(this.engineRenderer.ctx.provider.getSelection());
+  }
+
+  d2s() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:56:49Z

src/ui/CanvasTextureRenderer.ts

+
+      for (let i = fromIndex; i <= toIndex; i++) {
+        if (this.engineRenderer.ctx.provider.isSelected(this.currentLocalData[0][i].i)) {
+          ctx.fillStyle = '#ffa809';


Use class color constant

thinkh · 2018-10-29T11:56:53Z

src/ui/CanvasTextureRenderer.ts

+        if (this.engineRenderer.ctx.provider.isSelected(this.currentLocalData[0][i].i)) {
+          ctx.fillStyle = '#ffa809';
+        } else {
+          ctx.fillStyle = '#ffffff';


Use class color constant

thinkh · 2018-10-29T11:58:57Z

src/ui/taggle/TaggleRenderer.ts

    this.renderer.enableHighlightListening(enable);
  }
+
+  d2s() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:59:01Z

src/ui/taggle/TaggleRenderer.ts

+    this.renderer.d2s();
+  }
+
+  s2d() {


Do not use abbreviations in this case.

thinkh · 2018-10-29T11:59:36Z

src/ui/toolbar.ts

    const others = order.filter((d) => !ss.has(d));
    ctx.provider.setSelection(others);
+  }),
+  selectionToOverviewDetail: ui('S2D', (_col, _evt, ctx, level) => {


Use more explainable label

thinkh · 2018-10-29T11:59:42Z

src/ui/toolbar.ts

+    const s = ctx.provider.getSelection();
+    ctx.provider.setDetail(s);
+  }),
+  overviewDetailToSelection: ui('D2S', (_col, _evt, ctx, level) => {


Use more explainable label

thinkh · 2018-10-29T12:02:11Z

src/model/OverviewDetailColumn.ts

+
+  group(row: IDataRow) {
+    const isSelected = this.getValue(row);
+    return isSelected ? OverviewDetailColumn.DETAILED_GROUP : OverviewDetailColumn.NOT_DETAILED_GROUP;  }


Check formatting

thinkh · 2018-10-29T12:23:19Z

src/ui/CanvasTextureRenderer.ts

+  private alreadyExpanded: boolean = false;
+  private expandLaterRows: any[] = [];
+  private readonly options: Readonly<ILineUpOptions>;
+  private readonly idPrefix = 'testprefix';


Why testprefix?

thinkh · 2018-10-29T12:24:09Z

src/ui/CanvasTextureRenderer.ts

+      totalWidth += rankingWidth;
+    });
+    if (totalWidth > this.node.clientWidth) {
+        this.currentNodeHeight -= 20;


Where does this 20 px are coming from? Is there an existing constant that you can use?

thinkh

I couldn't test this functionality with the lineupjs demos, as there is a bug (as described above). Afterwards I can test it again.
Please fix the mentioned comments, before I'll merge the PR.

domdir added 6 commits June 24, 2018 22:47

implementation of a canvas texture renderer, for Caleydo/lineup_app#3 …

581e36b

…and Caleydo/lineup_app#4

implementation of the canvas texture renderer, for Caleydo/lineup_app#3

ca0dcaa

Caleydo/lineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

added OverviewDetail column, for Caleydo/lineup_app#3 Caleydo/lineup_…

957530f

…app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

Merge branch 'taggle-fusion' into taggle-fusion-issue-3-4

267cc37

added overviewDetail to SidePanel, for Caleydo/lineup_app#3 Caleydo/l…

391fd54

…ineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

splitting big canvas into chunks and minor fixes, for Caleydo/lineup_…

80fc751

…app#3 Caleydo/lineup_app#4 Caleydo/lineup_app#5 Caleydo/lineup_app#6

domdir requested a review from thinkh September 7, 2018 09:01

thinkh changed the title ~~Taggle fusion issue 3 4~~ Taggle Fusion Canvas Texture Renderer Sep 7, 2018

bug fixes, for Caleydo/lineup_app#20 Caleydo/lineup_app#21 Caleydo/li…

56c676a

…neup_app#22 Caleydo/lineup_app#23

domdir force-pushed the taggle-fusion-issue-3-4 branch from a636edd to 56c676a Compare October 7, 2018 18:00

domdir and others added 8 commits October 22, 2018 18:01

code clean up

f1a760d

Merge branch 'taggle-fusion' into taggle-fusion-issue-3-4

ca0a8fa

Merge branch 'taggle-fusion' into taggle-fusion-issue-3-4

2fd3e96

# Conflicts: # src/model/index.ts

Fix typo

43c240f

Fix compile error

7aa6ec4

Add d3-select and d3-drag packages

14ea9ca

Fix possible null references for ownerDocument

053a2b9

Fix scsslint

bc75d66