Ignore Code type cells in horizontalScrollbar #24576
Conversation
Charles-Gagnon
requested review from
corivera,
chlafreniere and
a team
and removed request for
corivera and
chlafreniere
| @@ -355,7 +355,7 @@ export class CodeComponent extends CellView implements OnInit, OnChanges { | |||
| horizontalScrollbar.style.top = horizontalTop + 'px'; | |||
| horizontalScrollbar.style.bottom = ''; | |||
| } | |||
| } else { | |||
| } else if (this.cellModel.cellType !== CellTypes.Code) { | |||
| // If horizontal scrollbar is not needed then set do not show it | |||
| horizontalScrollbar.style.opacity = '0'; | |||
There was a problem hiding this comment.
Why are we even using opacity to hide the scrollbar? I would think display: none would be the better option
There was a problem hiding this comment.
Let's make sure we aren't regressing the scenarios this was originally added for too : #17083
There was a problem hiding this comment.
One thing to keep in mind is the word wrap setting in all of these code paths
There was a problem hiding this comment.
I believe that display:none could be used here so that the dom layout would will not need to compute the layout for the horizontal scrollbar. I believe when I was testing all the paths it was much easier to have this change so that I was able to still see the scrollbar in the DOM in order to change it if necessary.
@unlock21 please change to using display instead :)
There was a problem hiding this comment.
@VasuBhog changed it to use display.
You all got to this way faster than I'd expected, thank you!
|
@corivera would you mind taking a look at this branch and validating that changes here look good? |
|
Sure |
|
@corivera have you had a chance to look at this PR? Do you think this is something we should merge? |
|
@kburtram The PR is a year old, so someone would have to investigate whether the changes still work as intended. |
|
I'm going to close this as abandoned since I don't have anyone available to verify this won't regress other notebooks scenarios, and the notebook dev team hasn't been able to verify & merge in the past year. |
#21145
Ignore Code Cells being applied with Markdown Cell specific behavior around the horizontal scrollbar.
The scrollbar is already there, just opacity was set to 0.
Before:
After:
