Dashboard improvement (necessary merge of #1658 and #1676 + fix #1492) #1739

Merged
merged 22 commits into from May 31, 2012

Conversation

Projects
None yet
5 participants
@Carreau
Member

Carreau commented May 15, 2012

This is merge of #1658 (alternate upload) and #1676 (shutdown fron dashboard) that were conflicting, because #1658 (alternate upload) is necessary for #1676 (shutdown fron dashboard), otherwise you can't upload file as dashbord refresh just after drag and drop...

In addition :

One small bug fix of #1658.

fixes #1492 ( bigger drag target when no notebook), it show "list is empty" when no notebooks.

finally on master.

@ellisonbg

This comment has been minimized.

Show comment
Hide comment
@ellisonbg

ellisonbg May 23, 2012

Member

@fperez, can you test this branch on Linux to see if this fixes the uploadability of notebooks? (in your vast spare time ;-)

Member

ellisonbg commented May 23, 2012

@fperez, can you test this branch on Linux to see if this fixes the uploadability of notebooks? (in your vast spare time ;-)

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez May 23, 2012

Member

I greatly appreciate the subtle sense of humor, @ellisonbg ;)

Sure, I'll try to take this for a spin tomorrow; I'm fried for the day. Thanks for reminding me...

Member

fperez commented May 23, 2012

I greatly appreciate the subtle sense of humor, @ellisonbg ;)

Sure, I'll try to take this for a spin tomorrow; I'm fried for the day. Thanks for reminding me...

@jenshnielsen

This comment has been minimized.

Show comment
Hide comment
@jenshnielsen

jenshnielsen May 25, 2012

Contributor

I have tested uploading notebooks via both the button and drag and drop on linux (ubuntu 12.04) in Firefox and Chromium and it appears to be working fine in all cases.
The shutdown button also works as expected and is a great feature. One small issue that I see is that if you keep a notebook open
but shut it down from the dash and try to execute code in the notebook afterwards the restart kernel dialog pops up as expected but if you press restart the cell is not actually executed until you rerun it but it appears busy i.e it shows up as "In [*]:" until you rerun it.

Contributor

jenshnielsen commented May 25, 2012

I have tested uploading notebooks via both the button and drag and drop on linux (ubuntu 12.04) in Firefox and Chromium and it appears to be working fine in all cases.
The shutdown button also works as expected and is a great feature. One small issue that I see is that if you keep a notebook open
but shut it down from the dash and try to execute code in the notebook afterwards the restart kernel dialog pops up as expected but if you press restart the cell is not actually executed until you rerun it but it appears busy i.e it shows up as "In [*]:" until you rerun it.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau May 25, 2012

Member

I think this is a larger issue, this appends also if the kernel dies by itself.
Even more, you can save notebook with running prompt, and load them afterwards. They will still show cells with running prompt even if the new kernel is, of course, idle.

Member

Carreau commented May 25, 2012

I think this is a larger issue, this appends also if the kernel dies by itself.
Even more, you can save notebook with running prompt, and load them afterwards. They will still show cells with running prompt even if the new kernel is, of course, idle.

@jenshnielsen

This comment has been minimized.

Show comment
Hide comment
@jenshnielsen

jenshnielsen May 26, 2012

Contributor

@Carreau Yes you are right this change only makes it easier to expose the bug and I don't think that this
should hold back the merge of this pull request.

Contributor

jenshnielsen commented May 26, 2012

@Carreau Yes you are right this change only makes it easier to expose the bug and I don't think that this
should hold back the merge of this pull request.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau May 30, 2012

Member

I'll merge in a few hours if nobody as objection as it only adds some functionality and seem to work in linux.

Member

Carreau commented May 30, 2012

I'll merge in a few hours if nobody as objection as it only adds some functionality and seem to work in linux.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 30, 2012

Member

@Carreau : Let's give people about a day, to make sure people in California have had a chance to look at it, though I expect it will be all OK.

Member

takluyver commented May 30, 2012

@Carreau : Let's give people about a day, to make sure people in California have had a chance to look at it, though I expect it will be all OK.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez May 31, 2012

Member

I've confirmed that it works on Linux, and that indeed it allows uploading into Chrome when SSL is on. I double-checked that situation, and it looks like it's a chrome-specific thing (and could be just on linux), because with Firefox the d'n'd upload works fine. So this is a most welcome fix! Since it has already had good review otherwise, go for it.

My only suggestion would be to change the phrase in the dashboard

"Drag files onto the list, or click here, to import notebooks"

to

"To import a notebook, drag the file onto the listing below or click here."

I just think it reads better, that's all.

Member

fperez commented May 31, 2012

I've confirmed that it works on Linux, and that indeed it allows uploading into Chrome when SSL is on. I double-checked that situation, and it looks like it's a chrome-specific thing (and could be just on linux), because with Firefox the d'n'd upload works fine. So this is a most welcome fix! Since it has already had good review otherwise, go for it.

My only suggestion would be to change the phrase in the dashboard

"Drag files onto the list, or click here, to import notebooks"

to

"To import a notebook, drag the file onto the listing below or click here."

I just think it reads better, that's all.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau May 31, 2012

Member

Ok, changed, merging then.

Member

Carreau commented May 31, 2012

Ok, changed, merging then.

Carreau added a commit that referenced this pull request May 31, 2012

Merge pull request #1739 from Carreau/dashboardImprovement
Dashboard improvement

see #1658 #1676

Allow to shutdown the kernels from the dashboard, 
autorefresh dashboard,
add a native upload method, especially for https/chrome/linux that prevent drag and drop

@Carreau Carreau merged commit 2caea25 into ipython:master May 31, 2012

@jenshnielsen

This comment has been minimized.

Show comment
Hide comment
@jenshnielsen

jenshnielsen May 31, 2012

Contributor

Thanks for this improvement. I think a small bug appeared when the text was changed.
To me it seems like you only moved the test but did not change the position where one has to click
to open the file dialog. I have to click on either the or listing now to open the file dialog.

Contributor

jenshnielsen commented May 31, 2012

Thanks for this improvement. I think a small bug appeared when the text was changed.
To me it seems like you only moved the test but did not change the position where one has to click
to open the file dialog. I have to click on either the or listing now to open the file dialog.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau May 31, 2012

Member

Can you try to flush your browser cache,(css is often cached longer that html). I did change the size of the click area.
Actually you should be able to to click anywhere on the whole sentence to have the dialog opening.

Thanks.

Member

Carreau commented May 31, 2012

Can you try to flush your browser cache,(css is often cached longer that html). I did change the size of the click area.
Actually you should be able to to click anywhere on the whole sentence to have the dialog opening.

Thanks.

@jenshnielsen

This comment has been minimized.

Show comment
Hide comment
@jenshnielsen

jenshnielsen May 31, 2012

Contributor

Yes I see. It works correctly in chrome. In firefox neither clearing the cache nor using a different user profile seems to work.

Contributor

jenshnielsen commented May 31, 2012

Yes I see. It works correctly in chrome. In firefox neither clearing the cache nor using a different user profile seems to work.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau May 31, 2012

Member

hum... right, no idea why, the firefox css inspector does say that the fileinput have the right size, but dont correctly respond to click.

@fperez , should we then get back to
"Drag files onto the list, or click here, to import notebooks"
to be more aligned with FF ?

Member

Carreau commented May 31, 2012

hum... right, no idea why, the firefox css inspector does say that the fileinput have the right size, but dont correctly respond to click.

@fperez , should we then get back to
"Drag files onto the list, or click here, to import notebooks"
to be more aligned with FF ?

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez May 31, 2012

Member

Mmh, I don't particularly like having to keep a contorted sentence to satisfy a bizarre browser bug... I guess if it's broken we do need to fix it, but in that case please open an issue about this so that we can return later to the problem (or somebody who knows more about browsers can). I find it truly strange that word order can affect clicking response...

From experimenting a bit with FF, it seems that it only responds to clicks in the area from the start of the sentence up to around 260 pixels, which is ~58% of the 447px input area.

So I suggest instead the following wording:

"To import a notebook, click here or drag a file onto the list below:"

Hopefully that will work.

Member

fperez commented May 31, 2012

Mmh, I don't particularly like having to keep a contorted sentence to satisfy a bizarre browser bug... I guess if it's broken we do need to fix it, but in that case please open an issue about this so that we can return later to the problem (or somebody who knows more about browsers can). I find it truly strange that word order can affect clicking response...

From experimenting a bit with FF, it seems that it only responds to clicks in the area from the start of the sentence up to around 260 pixels, which is ~58% of the 447px input area.

So I suggest instead the following wording:

"To import a notebook, click here or drag a file onto the list below:"

Hopefully that will work.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #1739 from Carreau/dashboardImprovement
Dashboard improvement

see #1658 #1676

Allow to shutdown the kernels from the dashboard, 
autorefresh dashboard,
add a native upload method, especially for https/chrome/linux that prevent drag and drop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment