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

[Applab Datasets] Set Up Layout for new Data Tab #30331

Merged
merged 3 commits into from Aug 22, 2019
Merged

Conversation

ajpal
Copy link
Contributor

@ajpal ajpal commented Aug 20, 2019

Right now just the basic layout. Next step is the Data Browser content, then the Library Pane content. Everything behind an experiment flag.

With Experiment on:
image

With Experiment off:
image

@codecov-io
Copy link

codecov-io commented Aug 20, 2019

Codecov Report

Merging #30331 into staging will decrease coverage by 1.14%.
The diff coverage is 83.72%.

Impacted file tree graph

@@             Coverage Diff             @@
##           staging   #30331      +/-   ##
===========================================
- Coverage    73.28%   72.13%   -1.15%     
===========================================
  Files         2050     1374     -676     
  Lines       112326    84689   -27637     
  Branches      3424     3427       +3     
===========================================
- Hits         82313    61091   -21222     
+ Misses       26773    20353    -6420     
- Partials      3240     3245       +5
Flag Coverage Δ
#integration 69.21% <83.72%> (+13.41%) ⬆️
#storybook 56.61% <100%> (ø) ⬆️
#unit ?
Impacted Files Coverage Δ
apps/src/util/experiments.js 97.4% <100%> (+0.03%) ⬆️
apps/src/storage/dataBrowser/DataBrowser.jsx 66.66% <100%> (ø)
apps/src/storage/dataBrowser/DataLibraryPane.jsx 68.42% <70%> (ø)
apps/src/storage/dataBrowser/DataWorkspace.jsx 85.71% <80%> (-3.18%) ⬇️
apps/src/storage/dataBrowser/DataOverview.jsx 83.58% <87.5%> (-0.63%) ⬇️
apps/src/templates/instructions/utils.js 78.78% <0%> (-7.58%) ⬇️
...board/config/initializers/new_relic_gc_profiler.rb
dashboard/app/mailers/pd/workshop_mailer.rb
... and 681 more

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 3847a0e...3032262. Read the comment docs.

@ajpal ajpal changed the title Set Up Layout for new Data Tab [Applab Datasets] Set Up Layout for new Data Tab Aug 21, 2019
borderRight: '1px solid gray',
overflowY: 'auto',
padding: 10,
paddingRight: 0 // setting this to 0 allows 2 columns with the potential scrollbar on Windows
Copy link
Contributor

Choose a reason for hiding this comment

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

In which cases is this needed? I pulled the branch really quick to test it out, and disabling this didn't cause a difference on IE or Chrome. Does this have a bigger effect when there is scrollable data?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might also be able to go without display: 'block'

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make more sense to make the width 250 and go without the border-box? Not sure which is clearer, but it's an option.

width: 270,
boxSizing: 'border-box',
borderRight: '1px solid gray',
overflowY: 'auto',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be 'hidden'?

bottom: 0,
left: 270,
right: 0,
boxSizing: 'border-box',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't need border-box here

Copy link
Contributor

Choose a reason for hiding this comment

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

(same with the container)

@ajpal ajpal merged commit 8672347 into staging Aug 22, 2019
@ajpal ajpal deleted the aug19-data-tab-tabs branch September 6, 2019 20:57
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.

None yet

3 participants