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

Executing cell when clicking its prompt #3535

Merged
merged 5 commits into from May 23, 2018

Conversation

Projects
None yet
2 participants
@lucasoshiro
Copy link
Contributor

lucasoshiro commented Apr 16, 2018

Closes #1009. Based on #3437. When clicking on a prompt, executes the corresponding cell.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Apr 16, 2018

(Repeating my comment from #1009)

Interesting idea! I have some concern that people might be used to clicking on the prompt to select a cell without going into edit mode, and if that suddenly executes the cells, it could be an unpleasant surprise - especially for cells which take a long time to run.

I'll try to think about how we might test it out.

@lucasoshiro

This comment has been minimized.

Copy link
Contributor Author

lucasoshiro commented Apr 18, 2018

Thanks for the feedback! I'll think about it

@lucasoshiro

This comment has been minimized.

Copy link
Contributor Author

lucasoshiro commented Apr 18, 2018

Hello again. Now, when the mouse is over a cell, a small run icon appears on the left of the prompt:

button_mouse_over

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Apr 18, 2018

I like this idea for mouse-driven use, though it doesn't help so much on a touchscreen. Maybe we can use some CSS media queries to make these buttons always visible on touch devices?

There's a test failure where a test checks the HTML of the input prompt.

@lucasoshiro

This comment has been minimized.

Copy link
Contributor Author

lucasoshiro commented Apr 26, 2018

I have searched a media query for touchscreen devices. I am using any-pointer: coarse. It worked on Chrome, but didn't worked on Firefox, maybe with some javascript it can be solved... I have also fixed that test failure, now the button is outside the prompt div.

@lucasoshiro

This comment has been minimized.

Copy link
Contributor Author

lucasoshiro commented May 5, 2018

@takluyver Sadly, any-pointer media query is not supported by some web browsers (https://caniuse.com/#search=any-pointer)... Do you think it's a good idea show the buttons always on those browsers?

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented May 6, 2018

It looks like it's mainly Firefox that doesn't support it, but there's a non-standard -moz-touch-enabled that works there. I found this suggested use on a bug report:

@media (-moz-touch-enabled: 1), (pointer: coarse) {
  // Rules for touch screens
}

https://developer.mozilla.org/en-US/docs/Web/CSS/@media/-moz-touch-enabled

@lucasoshiro

This comment has been minimized.

Copy link
Contributor Author

lucasoshiro commented May 19, 2018

That worked here, on a touchscreen computer. Thanks!

@takluyver
Copy link
Member

takluyver left a comment

I've tested this briefly interactively, and it looked good.

var run_this_cell = $('<div></div>').addClass('run_this_cell');
run_this_cell.prop('title', 'Run this cell');
run_this_cell.append('<i class="fa-step-forward fa"></i>');
run_this_cell.mouseup(function (event) {

This comment has been minimized.

@takluyver

takluyver May 21, 2018

Member

Is there a reason to do this on the mouseup event rather than the click event? I think we usually use click.

This comment has been minimized.

@lucasoshiro

lucasoshiro May 22, 2018

Author Contributor

Ok, I'll replace it with click event

@takluyver takluyver modified the milestones: 6.0, 5.5, 5.6 May 23, 2018

@takluyver takluyver merged commit 374da5a into jupyter:master May 23, 2018

4 checks passed

codecov/patch Coverage not affected when comparing 97b3b96...40cf1ab
Details
codecov/project 74.46% (-0.56%) compared to 97b3b96
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented May 23, 2018

Thanks, let's see how this works out. I may ping you if there are issues with it.

@mpacer mpacer changed the title [WIP] Executing cell when clicking its prompt Executing cell when clicking its prompt Jun 13, 2018

mpacer added a commit to mpacer/notebook that referenced this pull request Jun 14, 2018

add el & update css in output_area to match run button in In from jup…
…yter#3535

Without this change the output and Input areas become unaligned.

This still isn't an ideal fix, but it'll make a release possible without
making the styling problematic.

rgbkrk added a commit that referenced this pull request Jun 14, 2018

Merge pull request #3687 from mpacer/consistent_prompt
add el & update css in output_area to match run button in input prompt from #3535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.