react-native-walkthrough-tooltip icon indicating copy to clipboard operation
react-native-walkthrough-tooltip copied to clipboard

Child element does not fill parent height when using height: 100%

Open Saikedo opened this issue 2 years ago • 8 comments

It seems like the height attribute just gets ignored for the child of the toolTip if you use percentage height.

Let`s take a look at this example. Here the parent view has a height of 300 and I am setting the child component of the tooltip, The view and the text inside, to havea height of 100%. What I expect here to happen is that the view that wraps the text will span the entire height of the parent view but it seems like that '100%' gets ignored and the height of the inner view is determined by the height of the text inside.

<View style={{ width: 250, height: 300, backgroundColor: '#ff0000' }}>
  <Tooltip
    isVisible={showTip}
    content={
      <View>
        <Text> I am a tooltip </Text>
      </View>
    }
    onClose={() => setTip(false)}
    placement="top"
  >
    <View style={{ backgroundColor: 'yellow', width: '100%', height: '100%', flex: 1,}}>
      <Text>Child component</Text>
    </View>
  </Tooltip>
</View>

I tried everything that came to mind, playing with flex, flexGrow, and a bunch of other things but nothing worked. Basically, as soon as I use a numeric value for height it`s all good but this does not work with percentages?

Any ideas on how I can address this?

Thanks.

Saikedo avatar Jan 09 '22 05:01 Saikedo

Ah, I see the reason now. This is how react-native-walkthrough-tooltip renders the children element in the parent

{hasChildren ? (
    <View ref={this.childWrapper} onLayout={this.measureChildRect}>
      {children}
    </View>
  ) : null}

Since this wraps children element in a view that does not stretch to fill the parent height of the tooltip, the children height of '100%' will only fill the 100% of this new view height.

It`s probably a good idea to add a way to control the style of this view externally right ?

Saikedo avatar Jan 09 '22 05:01 Saikedo

Quite possibly. Do you want to do a PR for it?

gregfenton avatar Jan 09 '22 20:01 gregfenton

@gregfenton Sure, ill make a PR for that.

Just to make sure, we are just adding a way to pass styles to the original children wrapper correct? Since there is already a prop called childrenWrapperStyle, I think calling it originalChildrenWrapperStyle should be clear enough.

Also, are there any potential issues that I should be aware of? Just making sure adding styles to that view won`t mess up the layout calculations in any way.

Saikedo avatar Jan 09 '22 20:01 Saikedo

I think I'd consider calling it parentWrapperStyle given where it is going, and the fact that currently it is an empty value.

The above <View> would have style={[this.props.parentWrapperStyle]} ? If not provided, should not affect the styles at all?

gregfenton avatar Jan 09 '22 21:01 gregfenton

@gregfenton That's correct, empty by default so if parentWrapperStyle is undefined, then everything will stay the same.

Saikedo avatar Jan 09 '22 21:01 Saikedo

@gregfenton Just opened a PR.

Did some quick tests, did not notice any alterations to the original behavior if nothing is passed. And of course, now we are able to style the wrapper.

Quick question, should I also make a PR for #134 while I have the project open? Do you agree with the proposed solution there?

Saikedo avatar Jan 09 '22 21:01 Saikedo

Hi, I have a Pull Request with these fixes and was wondering if there is any ETA for merging this in. I am stuck using a local version of the library because we really needed this change for our project wanted to see if I can help with anything to hasten the merge.

Thank you.

Saikedo avatar Mar 09 '22 19:03 Saikedo

should this be closed now?

Csierram96 avatar Feb 02 '23 18:02 Csierram96