Rewrite structure in results.html

ronja-ui commented 8 months ago

To put it nicely, it's a hot mess.

The current structure is something like this:

<div class="paragraph-output">
  <dynamic-forms></dynamic-forms>
  <div class="table-display">
    <result-chart-selector.html>
      <div class="result-chart d-flex align-items-center flex-wrap">
        <div id="{{id}}_switch">...</div>
        <div id="{{paragraph.id}}_helium">...</div>
        <div class="dropdown me-2">(Graph data download button)</div>
        <div class="ms-auto">(Button for opening graph settings)</div>
      </div>
    <div id="p{{id}}_resize">
      <div ng-if="type=='TABLE' || type=='NETWORK'">
        <div class="option fw-semibold overflow-visible">(graph options)</div>
        <div class="mt-4 d-flex" id="p{{id}}_network_header">(network graph labels)</div>
        <div class="option fw-semibold overflow-visible">(graph settings)</div>
        <div id="p{{id}}_graph" class="graphContainer">
          <div id="p{{id}}_{{viz.id}}"></div>
        </div>
        <div id="p{{id}}_network_footer">...</div>
      </div>
      <div ng-if="type=='TEXT'" class="plainTextContainer">
        <div id="p{{id}}_text" class="text plainTextContent overflow-auto"></div>
      </div>
      <div ng-if="!isDefaultDisplay()"></div>
      <div ng-if="type=='ELEMENT'"></div>
      <div ng-if="type=='HTML'"></div>
      <div ng-if="type=='ANGULAR'"></div>
      <div ng-if="type=='IMG'"></div>
    </div>
    <div ng-repeat="app in apps">
      <div ng-show="config.helium.activeApp == app.id"></div>
    </div>
  </div>
  <div class="error">error message</div>
</div>

The ideal would be as simple as possible and prevent unnecessary ng-repeating with hidden content.

ronja-ui commented 8 months ago

Graphs have a similar problem as in #216, there's two different ids just for one graph: p{{id}}_graph and p{{id}}_{{viz.id}}

It shouldn't need two separate ids where one is for a container div and another one for the graph itself.

ronja-ui commented 8 months ago

Nested divs around plain text content has been removed succesfully.

I also moved succesfully graph settings from result.html to result-chart-selector.html and finally fixed the margin problem around graph settings.

ronja-ui commented 8 months ago

There's visible divs which shouldn't be visible in source code.

When a paragraph doesn't have any results, it should look something like this:

<div class="paragraph">
  <div ng-include="" src="'/app/notebook/paragraph/paragraph-control.html'">(paragraph controls)</div>
  <div class="paragraph-editor">...</div>
  <div class="paragraph-footer">...</div>
</div>

In other words, paragraph-output shouldn't appear in source code at all. Tho now it looks like this:

<div class="paragraph">
  <div ng-include="" src="'/app/notebook/paragraph/paragraph-control.html'">(paragraph controls)</div>
  <div class="paragraph-editor">...</div>
  <div class="paragraph-output">
    <dynamic-forms>...</dynamic-forms>
    <div class="error"></div>
  </div>
  <div class="paragraph-footer">...</div>
</div>

Outcome is a bit messy but the worst part is that it creates ghost margins because of those empty divs.

ronja-ui commented 8 months ago

A similar problem when there's results available.

paragraph-output looks like this for angular content:

<div class="paragraph-output">
  <dynamic-forms>...</dynamic-forms>
  <div class="table-display">
    <div ng-include="" src="'/app/notebook/paragraph/result/result-chart-selector.html'">...</div>
    <div id="p{{id}}_resize">
      <div ng-if="type == 'ANGULAR'">(results)</div>
    </div>
  </div>
</div>

At least <dynamic-forms> and result-chart-selector.html shouldn't be visible at all. This happens with other content (excluding graphs and dynamic forms content) as well.

BVVLD commented 6 months ago

Current status of the issue: the results module getting a full refactoring, which will include an optimized HTML structure, which will no longer create unnecessary containers, and combine result-chart-selector.html and settings panels inside the new result container itself.

BVVLD commented 5 months ago

Second iteration of refactor prototype is now available for testing in QA. Currently implemented viz modules: table, barchart, piechart, html. The code is still rough, bugs are expected.

BVVLD commented 5 months ago

Current module implementation status:

[x] Table
[x] Bar Chart
[x] Pie Chart
[x] Line Chart
[x] Area Chart
[x] Scatter Chart
[x] Network
[x] HTML
[x] Text
[x] Angular

BVVLD commented 5 months ago

Markdown and Angular forms are working fine.

BVVLD commented 5 months ago

Current bugs:

[x] NVD3 charts are adding empty svg tag every render, which is useless
[x] Sometimes Table type is rendered as Text during paragraph run, and doesn't switch automatically until reload of the page.
[x] Raw-data table doesn't populate placeholder table
[x] Linechart is broken with some config issues
[x] %dpl with normal tables sometimes produce a hickups and not load automatically before restart
[x] Graph settings responsiveness is bad, no immediate updates on user's actions
[x] Scatter chart settings needs fixing

BVVLD commented 5 months ago

Additional TODOs:

[x] Add the download button logic and hide it on errors
[x] Refactor Table module to use less unnecessary legacy code
[ ] Refactor date formatting in linechart mode
[ ] Create a dynamic module loader/importer instead of current, not-so-flexible implementation
[ ] Remove settings from report view
[x] Make network graph switchable only to table
[ ] Make network settings auto-save
[x] Clean graph navbar HTML structure with correct ng-if hiding
[x] Add append_paragraph logic
[x] Add additional settings logic bindings