Add clear history button #1119
Add clear history button #1119
Conversation
|
There's a z-order issue with tooltips in the eval frame (which I think maybe we've never had before...?)
Any thoughts? |
|
@mdboom I think the position of that tooltip could be place so that it appears in the lower right side of the icon. At the moment, the editor pane is overlapping a tooltip that is actually appearing in the eval frame. I'm not sure how we address this with this current setup, but it does seem like one solution is to just prevent the tooltip from being in the overlapping area to the best of our abilities. Setting the tooltip placement (outlined here) to |
There was a problem hiding this comment.
I think this is on the right track, but I'm noticing that there's some strange overflowing issues if I run a lot of cells. I tried a few less hacky versions of the below and they all seem to scoot the toolbar off the pane slowly.
Let me know if you need any help getting to the bottom of this!
|
Oh, CSS is such a joy! I'll poke at it and let you know if I get stuck. |
|
I think I've fixed the tooltip and shrinking height issues. |
|
LGTM - thanks @mdboom! |
| } | ||
| } | ||
|
|
||
| export function mapStateToProps() { |
There was a problem hiding this comment.
this mapStateToProps is not needed
https://stackoverflow.com/questions/47657365/can-i-mapdispatchtoprops-without-mapstatetoprops-in-redux
| return { } | ||
| } | ||
|
|
||
| function mapDispatchToProps(dispatch) { |
There was a problem hiding this comment.
preferred method is to just select out the actions you need, as in:
we have this pattern still floating around, but it is legacy
| return { } | ||
| } | ||
|
|
||
| const MiniToolbar = connect(mapStateToProps)(MiniToolbarUnconnected) |
There was a problem hiding this comment.
this does not appear to need to be connected to the redux store at all, right? it's just a simple presentational component?
|
Thanks for the more thorough review @bcolloran. I think I dropped the ball on this PR review + testing more thoroughly on Chrome. |
|
Sorry about this, all. I still feel like a monkey on a typewriter with React. Is there a good reference / book? |
|
No worries @mdboom!! :-) We're here to help you through it. Unfortunately we have a bunch a legacy code from when we were first learning that demonstrates bad patterns, and in many places it's not a great foundation to learn from... so really it's me and hamilton who must apologize to you! I don't know of any particular books, but i think the official react and redux docs are excellent. I've also found quite a number of great hackernoon and medium articles that have helped a lot when googling around specific topics. |
Fixes #1015 and #1090