Skip to content

Fix/fix notebook settings - #423

Closed
corneadoug wants to merge 5 commits into
apache:masterfrom
corneadoug:fix/fixNotebookSettings
Closed

Fix/fix notebook settings#423
corneadoug wants to merge 5 commits into
apache:masterfrom
corneadoug:fix/fixNotebookSettings

Conversation

@corneadoug

Copy link
Copy Markdown
Contributor

Setting Window used to be shown inside the notebook and therefore would scroll with notebook content.
In this PR, we are fixing it on top with the NotebookAction bar.
screen shot 2015-11-12 at 5 13 29 pm

We also have a scrollbar in the settings in case the window height is too small
screen shot 2015-11-12 at 5 46 10 pm

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

Ready To merge
(CI fails on flink, and I would rebase before merge)

I'd love to have a few feedbacks or review

@Leemoonsoo

Copy link
Copy Markdown
Member

I have tested. and working nicely.
One suggestion. How about limit the max-height of setting panel to about 3/4 - 4/5 size of the browser window height? That would clearly indicate user that setting panel is above the notebook.

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

I guess 4/5 in that case would work nicely. I will try and post a screenshot

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

@Leemoonsoo Like this?
screen shot 2015-11-13 at 10 56 37 am
If it's good I will push that new commit.
However we might want to rethink the layout of those settings later.

@Leemoonsoo

Copy link
Copy Markdown
Member

Yes, that's what i thought.

@Leemoonsoo

Copy link
Copy Markdown
Member

one small detail is

image

Isn't it make more sense to move shadow to the bottom of the setting panel, when it is expanded?

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

I'm trying to play with flexbox to get this result
screen shot 2015-11-13 at 12 03 35 pm

But it's not working on IE with max-height... so I'm trying to find a way

@bzz

bzz commented Jan 5, 2016

Copy link
Copy Markdown
Member

Looks great.
@corneadoug did you manage to find a way to do it?

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

I'm still stuck with IE and wasn't able to work on it since.
Since its an improvement its not urgent

@Leemoonsoo

Copy link
Copy Markdown
Member

Any update on it?

@asfgitasfgit closed this in c38a0a0May 9, 2018
asfgit pushed a commit that referenced this pull request May 9, 2018
close#83close#86close#125close#133close#139close#146close#193close#203close#246close#262close#264close#273close#291close#299close#320close#347close#389close#413close#423close#543close#560close#658close#670close#728close#765close#777close#782close#783close#812close#822close#841close#843close#878close#884close#918close#989close#1076close#1135close#1187close#1231close#1304close#1316close#1361close#1385close#1390close#1414close#1422close#1425close#1447close#1458close#1466close#1485close#1492close#1495close#1497close#1536close#1545close#1561close#1577close#1600close#1603close#1678close#1695close#1739close#1748close#1765close#1767close#1776close#1783close#1799
Sign up for freeto 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.

3 participants

@corneadoug@Leemoonsoo@bzz