-
Notifications
You must be signed in to change notification settings - Fork 555
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
Add "Show/Hide Inputs" button to navbar #681
Conversation
Original idea from http://chris-said.io/2016/02/13/how-to-make-polished-jupyter-presentations-with-optional-code-visibility/ Tweaked in http://mindtrove.info/code-hiding-on-nbviewer/. Why not in nbviewer itself?
Nice |
cc @michaelpacer who is working on tags. I'm going to guess that sometime you don't want to hide all the code cells . I have no objections. |
Agreed, @Carreau. I think this can grow over time to support tags for shown/hidden cells, the dashboard layout metadata , and other info stored in the notebook about how to present it. I've seen users get good mileage with the snippets linked above, and figured they would be a good starting point. |
Correcting a "at" above: /cc @mpacer |
Is there a reason you did this with js and not pure css animations (ala https://github.com/mpacer/hiding_tags_nbconvert/blob/master/toggle_hidden.tpl)? also, is there a reason to not have individual I hide buttons on the margin in addition to the global button? It could be hidden until hover which would harm discoverability but would surface exactly the feature you want by default (by instantly toggling the state for all code |
Only that I had working JS code to submit. I'm in favor of using a pure CSS approach. I'll look at updating this PR.
Same reason as above mainly. Plus I'm a fan of building things incrementally with user feedback. |
The biggest advantage that JS could give over my pure CSS approach that still (almost entirely) uses the pure CSS approach would be to have the JS extract and then fix the height of the cells (rather than leaving them on Regardless, I think that it would be cleaner if you toggled a class and just had altered properties associated with the class in the actual CSS files (rather than a raw encoding). One of the advantages of that is that once it's based on just toggling a class, the button for toggling/untoggling individual cells is really straightforward to implement (especially since we have a working example of it). |
Thanks @mpacer for the pointers. Here's the new rendition using a CSS class a just a tiny bit of JS to toggle it in the nav bar. There's no nbconvert config in nbviewer at the moment, other than one for the slideshow resources. We can follow-up by adding one to get the per-cell checkboxes in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Still, I think we can make the animation nicer by not taking some of the shortcuts that the almost pure CSS approach affords.
So we sacrifice a bit of that purity for the sake of substantially smoother animations by working with what we're actually trying to animate (height
) rather than what we were allowed to animate using only CSS (max-height
). I think that's worth doing.
Also it's still crazy to me that CSS can't animate from auto
values…I'm sure it has something to do with computation. But you'd think that there'd be a low overhead way to pass that message back and allow animation from auto
values. Because otherwise, this would be way easier.
|
||
body.hide_input_cells div.input { | ||
overflow:hidden; | ||
max-height: var(--max-height-small); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than setting the max height, since you're using js, you can extract the height itself and use that as the basis for the animation.
That would avoid the shadowing effect that you see in your .gif.
Note: I wanted to do this, but unfortunately, you just can't do that in pure CSS. The problem has to do with the height being set to auto. But since you are using js, you aren't stuck with the auto value for the height. If the height is explicitly set in the DOM, you can then use the transitions to modify the height in a smooth manner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can give height a shot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
/* adjust the max-height of input cells */ | ||
body div.input { | ||
transition: max-height var(--in-time) var(--transition-path-in), padding .0s step-end; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can similarly use height here, but you'll need to explicitly access the height at the time (e.g., for each of the inputs grab $(elem_id).height()
) but you'll need to do that in the js.
<li> | ||
<a href="javascript:{{js}}" title="{{name}}" {%if accesskey %}accesskey={{accesskey}}{% endif %}> | ||
<span class="fa fa-{{icon}} fa-2x menu-icon"></span> | ||
<span class="menu-text">{{name}}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, but it might be worth expanding the totality of the js that's included…
maybe something like (note this won't work and may not be valid js
<script>
for (x in $.('hide_input_cells')){
$(x.id).height($(x.id).height())
}
</script>
Or should this go below since you're actually passing the javascript in there and there is minimal js in this actual macro…
NB: I've not seen this use of an href
or a
before, I'm intrigued.
@@ -14,6 +13,8 @@ | |||
{% endif %} | |||
{% endfor %} | |||
|
|||
{{ layout.head_js_icon("$('body').toggleClass('hide_input_cells');", "Show/Hide Inputs", "eye", "i") }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is where the above code might actually go (inside the first argument (currently "$('body').toggleClass('hide_input_cells');"
))…
I actually looked this up a bit more and think these are most of the necessary details though there are still things that need to be checked… E.g., If you can do this without explicitly iterating over all the elements that need to be toggled.
I'll look at this tomorrow after sleeping on it.
for (x in $.('.input_area')){//input areas are the only things that have the class input area
x.height(x.height()); //first call sets it, second call retrieves it, they may need to be separated
// the key insight is that is that x.height() should return the pixel height and x.height(val) sets the height to that val.
x.toggleClass('hide_input_cell');//applies the toggle
}
Right now, I think you're applying 'hide_input_cell' to the entire body, but I figure this would be better to apply to the individual input areas. It also makes it a lot easier to open the way to have checkboxes in the future if can toggle individual cells. Of course, that means that the logic I wrote above won't work for the hide/show all button because then they can be toggled individually and this will just swap state. That's probably going to need more than one line though.
Is it possible that this is an optical illusion? Or are the pixels actually
doing that shadowing?
Because that should have worked as far as I can understand what is
happening.
If it has to do with line height… one possibility is to first squash all
text into one line. Then you have a common place to be working from. But I
think a simple drag/squash overlay would look awkward. It wouldn't look
good with fonts of different sizes and styles. I don't know if it would
look good for even one font. But it (or something like it) would solve the
irregularity by reducing everything to a common case.
I'll read more about css animations this week and get back to you. It
should be possible to make the animation smooth and continuous for any size.
…On Fri, Mar 24, 2017 at 19:28 Peter Parente ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nbviewer/static/less/notebook.less
<#681 (comment)>:
> + padding-top: var(--padding-hidden);
+ padding-bottom: var(--padding-hidden);
+}
+
+/* adjust the max-height of input cells */
+body div.input {
+ transition: max-height var(--in-time) var(--transition-path-in), padding .0s step-end;
+ -webkit-transition: max-height var(--in-time) var(--transition-path-in), padding .0s step-end;
+ -moz-transition: max-height var(--in-time) var(--transition-path-in), padding .0s step-end;
+ -o-transition: max-height var(--in-time) var(--transition-path-in), padding .0s step-end;
+ max-height: var(--max-height-big);
+}
+
+body.hide_input_cells div.input {
+ overflow:hidden;
+ max-height: var(--max-height-small);
I see the same shadowing when animating the height by setting the
div.input height on load and adding height: 0px !important when
.hide_input_cells is applied.
[image: toggle-inputs-css]
<https://cloud.githubusercontent.com/assets/153745/24318760/1f770ce6-10e1-11e7-9d16-fdaabca099ae.gif>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#681 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACXg6D1hGz9NlASnCtbIzBzzwyE0yFPtks5rpHuygaJpZM4MWDLH>
.
|
Sorry for not responding sooner.
I believe it's shadowing. I can see the entire contents of the cell input appear over all of the content instantly, while the space to accommodate it all grows. Since we're heading back toward using more JS to fix these issues, does it make sense to go back to the original JS implementation? |
Returning to this PR, do folks think we try to work out the shadowing issues with CSS, go with a straight JS approach, or abandon it altogether for now and revisit when there's a final nbformat spec that we can code toward? |
I think it would follow that option. |
OK. Closing this out. |
Adds a button to toggle cell inputs to the navbar. Tries to collapse whitespace between visibly empty cells by removing padding and border while avoiding too much jitter and excessive animation. Super basic but very effective.
The original idea is from @csaid (http://chris-said.io/2016/02/13/how-to-make-polished-jupyter-presentations-with-optional-code-visibility/). I tweaked it a bit in http://mindtrove.info/code-hiding-on-nbviewer/. Any reason this shouldn't live in nbviewer itself?