Fix/hint position

[ethanshar]

Description

• I renamed Hint's component internal methods to make it a little more readable
• I fixed the position of the Hint when the target is being pushed by another element - causing it to be misaligned and missed by our calculations.
  There's a dedicate method that calculates the offset for the hint message
• I added an example in HintScreen the showcase the issue so it's easier to check

Changelog

Fix Hint positioning when target is pushed by another element

Additional info

[nitzanyiz]

@ethanshar, Why did we fix this only for short messages?

[ethanshar]

@ethanshar, Why did we fix this only for short messages?

@nitzanyiz Two reasons

• If the message is long enough, there's no issue because the hint's tip is positioned ok with the hint
• If the message is long enough and we add it an offset it might get out of screen boundaries

[nitzanyiz]

I think most of the cases where we saw this bug is where they wanted to use the hint without a side tip and the target was on the sides of the screen. Currently after your fix, for short messages, it is fixed but for a little longer messages it not and it even get out of the screen. Not sure what is better 😅

import React, {Component} from 'react';
import {View, Text, Card, TextField, Button, Hint} from 'react-native-ui-lib';

export default class PlaygroundScreen extends Component {
  render() {
    return (
      <View bg-grey80 flex padding-20>
        <View style={{alignSelf: 'flex-start'}} bg-red30>
          <Hint message={'This is a short message'} visible useSideTip={false}>
            <Text>Hello</Text>
          </Hint>
        </View>
      </View>
    );
  }
}

[ethanshar]

<View bg-grey80 flex padding-20>
        <View style={{alignSelf: 'flex-start'}} bg-red30>
          <Hint message={'This is a short message'} visible useSideTip={false}>
            <Text>Hello</Text>
          </Hint>
        </View>
      </View>

Ok, we'll have to improve the logic. The assumption I did for short message is not good enough, I'll check if we can have a better solution

The problem is that your use case should have worked ok, maybe it's a different bug

[ethanshar]

@nitzanyiz After something digging, what you are showing is totally different issue
Unfortunately we made a lot of assumptions when rendering side hints and there are a lot of factor that affect the hint position. For example if you'll pass onBackgroundPress callback it will work differently
In any case the recommended UI for side hint is the side tip, when users change it they break the UI

Back to the issue I have fixed, I see it requires the user pass useSideTip={false} and it's not trivial so Im trying to fix it

[ethanshar]

@nitzanyiz @Inbal-Tish
I did extra adjustments, please review

• I increased the threshold that determine if the target positioned on left/right to get a little more accurate results
• As for the issue Nitzan showed, it's a different issue - I fixed it by changing the default edgeMargins when the hint is not rendered in a modal (assuming the margins are coming from the screen so we don't need to include them) - let me know if you need me to elaborate on that

I think these fixes solve most of the issues, for some cases where users do "stupid" things we can always fix but we can guide them which props to pass and how (useSideTip, edgeMargins, etc..)

[Inbal-Tish]

This simple example doesn't look good:
<View row center> <Hint visible={this.state.showHint} message={'xx'}> <Button label="Button" onPress={() => this.setState({showHint: !this.state.showHint})}/> </Hint> </View>
I think we need to set minimum width

[ethanshar]

This simple example doesn't look good: <View row center> <Hint visible={this.state.showHint} message={'xx'}> <Button label="Button" onPress={() => this.setState({showHint: !this.state.showHint})}/> </Hint> </View> I think we need to set minimum width

Yes, it also happens in master it's a different issue, it's because you don't pass onBackgroundPress
When not passing it, it doesn't render inside a modal and in that case we have all kind of issues

[ethanshar]

@Inbal-Tish @nitzanyiz
I pushed another fix with a min width as Inbal suggested, it fixes another edge case you found

Unfortunately, there's a root issue with this component which we need to consider refactoring
The component handles two use cases:

• Hint rendered in a modal (when onBackgroundPress is passed)
• Hint rendered in the screen of the target

The latter is a more complex scenario because we have scenarios where the hint's target is deeply nested which force us to do all kind of calculations to make the hint appear in the right location.
Because the Hint's code covers both cases there's no good separation and fixing an issue in one use case usually affects the other use case.
My suggestion for future refactor is to separate the Hint into two components: HintInModal, HintInScreen and separate the calculations as well.

Anyway, for now, I believe this PR fix most of issues users complained on so we can merge it (unless you find other regression issues)
Let me know what you think