[Explore][Adhoc Filters] Expanding the Adhoc Filter popover when the content expands #5032
Conversation
gabe-lyons
force-pushed
the
gabe_expanding_simple_tab
branch
from
ee84ffc
to
209ad80
Compare
Codecov Report
@@ Coverage Diff @@
## master #5032 +/- ##
=======================================
Coverage 77.35% 77.35%
=======================================
Files 44 44
Lines 8677 8677
=======================================
Hits 6712 6712
Misses 1965 1965Continue to review full report at Codecov.
|
| @@ -78,6 +79,13 @@ export default class AdhocFilterEditPopover extends React.Component { | |||
| document.removeEventListener('mousemove', this.onMouseMove); | |||
| } | |||
|
|
|||
| adjustHeight(heightDifference) { | |||
| this.setState(state => ({ | |||
| ...state, | |||
There was a problem hiding this comment.
Do you need this? I thought you could just return the update to state.
There was a problem hiding this comment.
yeah I think this can be omitted.
There was a problem hiding this comment.
good call fixing now
| adjustHeight(heightDifference) { | ||
| this.setState(state => ({ | ||
| ...state, | ||
| height: state.height + heightDifference, |
There was a problem hiding this comment.
Just curious, is there a specific reason to add the height difference instead of just strictly setting the new height?
There was a problem hiding this comment.
FWIW the re-resizable library I'm using for the new dashboard passes a delta.
There was a problem hiding this comment.
The big issue is that there are two things that can resize the popover ->
-
the user dragging the box
-
the input component expanding
tracking deltas just simplified dealing with both of those
|
@GabeLoins a few questions but lgtm otherwise! |
| @@ -163,6 +188,12 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon | |||
| } | |||
| } | |||
|
|
|||
| multiComparatorRef(ref) { | |||
| if (ref) { | |||
There was a problem hiding this comment.
are there cases where you don't get a ref? that seems strange but mostly curious.
There was a problem hiding this comment.
I don't understand why but I've seen refs come in as undefined sometimes
There was a problem hiding this comment.
thanks for fixing this!
lgtm after @michellethomas 's question about ...state
gabe-lyons
force-pushed
the
gabe_expanding_simple_tab
branch
from
209ad80
to
2823332
Compare
gabe-lyons
force-pushed
the
gabe_expanding_simple_tab
branch
from
2823332
to
d68c718
Compare
(cherry picked from commit a746fce)
mistercrunch
added
🏷️ bot
Previously to this PR, when adding/viewing a simple filter with number of comparators this could happen:
That doesn't look so great and also makes it quite annoying to make any changes because you are forced to manually expand the box to hit save. In this PR I have the popover expand in reaction when the input field expands.
test plan:
reviewers:
@michellethomas @john-bodley @williaster @mistercrunch