Skip to content

Conversation

davidanthoff
Copy link
Collaborator

@davidanthoff davidanthoff commented Jun 23, 2020

@arnavgautam I think we're close here! I did two main things,

(1) added a bunch of CSS to our work to mesh with what Material UI does (I put some links in the top of the CSS file for notes on where I found info)
(2) did a very preliminary Material UI theme adoption to get the font size down (https://codesandbox.io/s/l48vjmk3om?file=/index.js; https://codesandbox.io/s/l48vjmk3om?file=/index.js) but this could probably be a lot more elegant :)

Some potential TODOs:

  • make the data view list item go blue or orange when selected (we'd have to change them from list items to buttons or something (probably not worth too much effort on this first pass we can just make it a TODO for later especially since the graph title makes it clear what is selected)

  • decide if we want the scroll bars to be always visible (that's the current state, see top of the CSS file)

  • alter Material UI in a more elegant way ... although I don't really feel motivated to do this right now since it looks good as is ...

  • any other changes you think would make it look better! We can show it at the meeting and see if people want something different!

@davidanthoff davidanthoff marked this pull request as draft June 23, 2020 21:44
@davidanthoff davidanthoff added this to the Backlog milestone Jun 23, 2020
@lrennels lrennels mentioned this pull request Jun 26, 2020
@lrennels lrennels modified the milestones: Backlog, v1.1.0 Jun 27, 2020
@codecov
Copy link

codecov bot commented Aug 3, 2020

Codecov Report

Merging #719 (9ceb00f) into master (5441958) will increase coverage by 0.00%.
The diff coverage is 79.45%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #719   +/-   ##
=======================================
  Coverage   78.73%   78.74%           
=======================================
  Files          35       35           
  Lines        2996     3044   +48     
=======================================
+ Hits         2359     2397   +38     
- Misses        637      647   +10     
Flag Coverage Δ
unittests 78.74% <79.45%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/explorer/explore.jl 58.73% <40.00%> (-8.58%) ⬇️
src/explorer/buildspecs.jl 94.73% <90.62%> (-0.03%) ⬇️
src/core/defs.jl 82.68% <100.00%> (-0.24%) ⬇️
src/core/instances.jl 76.03% <100.00%> (+0.19%) ⬆️
src/core/model.jl 84.21% <100.00%> (+1.35%) ⬆️
src/core/paths.jl 80.28% <100.00%> (+3.23%) ⬆️
src/core/types/model.jl 88.88% <100.00%> (+1.38%) ⬆️
src/utils/getdataframe.jl 75.29% <100.00%> (+0.59%) ⬆️
src/core/defcomp.jl 82.35% <0.00%> (+0.69%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5441958...9ceb00f. Read the comment docs.

Copy link
Collaborator

@lrennels lrennels left a comment

Choose a reason for hiding this comment

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

@arnavgautam this is looking great! I made a few comments and will now push a PR addressing them. I can also work on adding more tests for the functions we are adding.

@lrennels lrennels marked this pull request as ready for review November 21, 2020 10:27
@lrennels lrennels merged commit 1ee67b5 into master Dec 3, 2020
@lrennels lrennels deleted the reactexplorer branch March 17, 2021 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants