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

DATA VIZ: Fix bugs, incorrect string and null displays #871

Merged
merged 11 commits into from
Sep 2, 2019

Conversation

lumos309
Copy link
Contributor

@lumos309 lumos309 commented Aug 16, 2019

  • Fixed null slash in head of pair not displaying
  • Fixed wonky arrows like this
  • Added quotation marks around strings (VIZ: draw_data improvements  #475)
  • Fixed incorrect viewport sizing for drawings with functions

@coveralls
Copy link

coveralls commented Aug 16, 2019

Pull Request Test Coverage Report for Build 3374

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 36.883%

Totals Coverage Status
Change from base Build 3372: 0.0%
Covered Lines: 2681
Relevant Lines: 6503

💛 - Coveralls

@martin-henz
Copy link
Member

Still not quite right: Now numbers appear as strings:

Screenshot 2019-08-18 at 12 52 57 AM

@lumos309
Copy link
Contributor Author

Still not quite right: Now numbers appear as strings:

Screenshot 2019-08-18 at 12 52 57 AM

Oops! Fixed now.

@geshuming
Copy link
Contributor

I'm getting Line 2: ReferenceError: is_array is not defined from this snippet:

const a = (pair(1, 2));
draw_data(pair(a, a));

@geshuming
Copy link
Contributor

Can you add the relevant functions that were removed from this commit

Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

Checked the recent bugs: all working.

Also canvas size is adapting to the size of the drawing.

@geshuming
Copy link
Contributor

Some more tests I performed...

const ebst = make_empty_tree();

const bstll = make_tree(4, ebst, ebst);
const bstlr = make_tree(5, ebst, ebst);

const bstrl = make_tree(6, ebst, ebst);
const bstrr = make_tree(7, ebst, ebst);

const bstl = make_tree(2, bstll, bstlr);
const bstr = make_tree(3, bstrl, bstrr);

const bst = make_tree(1, bstl, bstr);


draw_data(bst);

results in:

download2

Note the non-uniform padding of the image.


const ist = list(1, "abc", x => x, pair(1,2), null, "superlongstring", 1234567890);

draw_data(ist);

results in:

Capture

Note the *1 where "superlongstring" is supposed to be.


The data viz component doesn't resize together with the parent component (see below).

Capture2

It seems like the component has a fixed window height? If the window is overflowed, it would result in 2 scroll bars!!!

image

@geshuming
Copy link
Contributor

Merging due to regression issues

@geshuming geshuming merged commit 25d432d into master Sep 2, 2019
@geshuming geshuming deleted the data-visualizer branch September 2, 2019 15:23
wltan pushed a commit to Source-Academy-Game/cadet-frontend that referenced this pull request Feb 19, 2020
…y#871)

* Fix bugs, incorrect string and null displays

* Fix numbers not displaying correctly

* Fix data visualizer breaking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants