You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by "Ian Maxon (Code Review)" <do...@asterixdb.incubator.apache.org> on 2018/07/05 23:02:50 UTC
Change in asterixdb[release-0.9.4-pre-rc]: [NO ISSUE] Asterixdb-dashboard baseline:
Ian Maxon has posted comments on this change.
Change subject: [NO ISSUE] Asterixdb-dashboard baseline:
......................................................................
Patch Set 4:
(14 comments)
Frankly I think the new design makes excessive use of expansion panels. There's not any use in requiring three layers of clicks to view the result of a query. Furthermore I don't understand the hiding of the dataverse inspector behind a hamburger button. Why is there such a drastic change from the original design?
Also, the way to retrieve the plans is either not functional or hidden so far I haven't found it yet; I haven't figured out from the code where it should be just yet.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/appbar.component.ts
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/appbar.component.ts:
Line 31: this.sideMenuVisible$ = this.store.select(s => s.app.sideMenuVisible);
tabs v. spaces
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/input.component.html
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/input.component.html:
PS4, Line 15: <mat-expansion-panel class="mat-elevation-z0">
: <mat-expansion-panel-header class="header">
Let's have this expansion panel be open by default; having to click to expand the input panel on first load seems very odd to me.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/input.component.ts
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/input.component.ts:
PS4, Line 35: queryString: string = "Use Metadata; select * from CompactionPolicy";
The box needs to be empty by default.
PS4, Line 72: if (data != undefined && data[this.currentQuery] === true){
: this.querySuccess = true;
: } else {
: this.querySuccess = false;
: }
:
We shouldn't mix tabs and spaces.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/output.component.ts
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/output.component.ts:
PS4, Line 29: @Component({
: moduleId: module.id,
: selector: 'awc-results',
: templateUrl:'output.component.html',
: styleUrls: ['output.component.scss']
: })
Tabs/spaces again
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.html
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.html:
PS4, Line 75: -<text class="operation-details" x="50%" y="50" text-anchor="middle">{{details}}</text> -->
Let's remove the commented code.
PS4, Line 78: !--<svg id='node{{level}}{{item}}{{subplan}}{{planName}}' xmlns="http://www.w3.org/2000/svg" xml:lang="en" class="dot"
: xmlns:xlink="http://www.w3.org/1999/xlink"
: width="10px" height="50px">
: <title>Connection</title>
: <circle cx="5" cy="5" r="5"/>
: <line x1="5" y1="10" x2="5" y2="50" stroke="blue" />
: <circle cx="5" cy="45" r="5"/>
: </svg> -->
Let's remove the commented code.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.scss
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.scss:
PS4, Line 100: // display: flex;
: // .title {
: // font-size: 10px;
: // font-weight: 500;
: // color: blue;
: // //padding-top: 12px;
: // }
: // }
: // .details {
: // padding-top: 10px !important;
: // padding-bottom: 20px !important;
: // margin: 0px !important;
: // overflow: auto;
: // font-size: 10px;
: // text-align: left;
: // visibility: hidden;
: // transition: opacity 1s ease-in-out;
: // opacity: 0;
: // }
Let's remove the commented code.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.ts
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node-svg.component.ts:
PS4, Line 88: // var mycircle = document.getElementById('node'+ this.level + this.item + this.subplan + this.planName);
:
: // mycircle.addEventListener('click', function(e) {
: // console.log('clicked - enlarging: ' + 'node'+ this.level + this.item + this.subplan + this.planName);
: // }, false);
:
: // let global = this.renderer.listen(mycircle, 'click', (evt) => {
: // console.log('Clicking the document', evt);
: // console.log('clicked - enlarging: ' + 'node'+ this.level + this.item + this.subplan + this.planName);
: // })
:
: // var svg = document.getElementById('mysvg'+ this.level + this.item + this.subplan + this.planName),
: // NS = svg.getAttribute('xmlns');
:
: // add random circles
: /*var c, i;
: for (i = 0; i < 1000; i++) {
: c = document.createElementNS(NS, 'circle');
: c.setAttributeNS(null, 'cx', Math.round(Math.random() * 100));
: c.setAttributeNS(null, 'cy', Math.round(Math.random() * 100));
: c.setAttributeNS(null, 'r', 1 ); // 20 + Math.round(Math.random() * 30));
: svg.appendChild(c);
: }*/
:
: /* click a circle
: svg.addEventListener('click', function(e) {
: var t = e.target;
: if (t.nodeName != 'circle') return;
: t.setAttributeNS(null, 'r',
: parseFloat(t.getAttributeNS(null, 'r')) + 10
: );
: console.log(t.getBoundingClientRect());
: }, false); */
: //global();
: // let simple = this.renderer.listen(this.myButton.nativeElement, 'click', (evt) => {
: // console.log('Clicking the button', evt);
: // });
: //simple();
: //Adding classes for further granularity/level styling
: //console.log('node'+ this.level + this.item + this.subplan + this.planName);
: //var svg = document.getElementById('node'+ this.level + this.item + this.subplan + this.planName);
: //svg.className.baseVal = ... or svg.setAttribute("class", ...) or svg.classList.add(...) or.
: //svg.classList.add("level" + this.level + "item" + this.item);svg.classList.add("subplan" + this.subplan);
: //svg.classList.add("planName" + this.planName);
Let's remove the commented code.
PS4, Line 223: /*
: console.log(me);
: console.log(this.viewParams_)
:
: if (this.viewParams_.viewMode === 'NORMAL') {
: this.viewParams_ = FULL;
: console.log('FULL')
: } else {
: this.viewParams_ = NORMAL;
: console.log('NORMAL')
: }
:
: var ele = document.getElementById('node'+ this.level + this.item + this.subplan + this.planName);
: (<any>ele).style.width = this.viewParams_.width;
: (<any>ele).style.height = this.viewParams_.height;
: console.log(ele);
: if (this.viewParams_.viewMode === "FULL") {
: (<any>ele).style.height = "unset";
: (<any>ele).style.maxHeight = this.viewParams_.height;
: }
: var ele2 = document.getElementById('node'+ this.level + this.item + this.subplan + this.planName).getElementsByClassName('operation-details');
: console.log(ele2);
:
: (<any>ele2[0]).style.visibility = this.viewParams_.visible;
: (<any>ele2[0]).style.opacity = this.viewParams_.opacity;
:
: var ele3 = document.getElementById('node'+ this.level + this.item + this.subplan + this.planName).getElementsByClassName('operation-details');
: (<any>ele3[0]).style.borderTop = this.viewParams_.border;
: console.log(ele3); */
Let's remove the commented code.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node.component.ts
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-node.component.ts:
PS4, Line 203: getElementById() Returns the element that has the ID attribute with the specified value
: getElementsByClassName() Returns a NodeList containing all elements with the specified class name
: getElementsByName() Returns a NodeList containing all elements with a specified name
: getElementsByTagName()
: changeElement(id) {
: var el = document.getElementById(id);
: el.style.color = "red";
: el.style.fontSize = "15px";
: el.style.backgroundColor = "#FFFFFF";
: }
:
: changeElementSize(id) {
: var el = document.getElementsByName('plan-node');
: console.log(el)
: // el.style.color = "red";
: // el.style.fontSize = "15px";
: // el.style.backgroundColor = "#FFFFFF";
: }
: */
Let's remove the commented code.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-view.component.html
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-view.component.html:
PS4, Line 26: <!-- <ul class="start">
: <li class='listart'>
: <plan-node [planName] = "planName" [node]="plan_" [level]="0" [item]="0" [subplan]="0"></plan-node>
: </li>
: </ul> -->
Let's remove the commented code.
https://asterix-gerrit.ics.uci.edu/#/c/2688/4/asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-view.component.scss
File asterixdb/asterix-dashboard/src/node/src/app/dashboard/query/plan-view.component.scss:
PS4, Line 69: //padding: 20px;
: //padding-right: 50px;
: //margin-right: 25px;
Let's remove the commented code.
PS4, Line 77: //border-left: 1px solid gray;
: padding-left: 25px; //display: inline-block;
: //margin: auto;
: }
Let's remove the commented code.
--
To view, visit https://asterix-gerrit.ics.uci.edu/2688
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic821d20ad927bdb209ea8ba531de926b2d6c7242
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: release-0.9.4-pre-rc
Gerrit-Owner: Emilio Jose Coronado Lopez <em...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-HasComments: Yes