Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tooltip Both Fix with Dynamic State #7288

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Tooltip Both Fix with Dynamic State #7288

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a903871
fix(tooltip): both fix with dynamic state
ddoemonn Oct 1, 2024
5cd39de
fix(tooltip): syntax error
ddoemonn Oct 1, 2024
1287449
fix(tooltip): group useState declarations and rename variable to mult…
ddoemonn Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion components/lib/tooltip/Tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const Tooltip = React.memo(
const [visibleState, setVisibleState] = React.useState(false);
const [positionState, setPositionState] = React.useState(props.position || 'right');
const [classNameState, setClassNameState] = React.useState('');
const [multipleFocusEvents, setMultipleFocusEvents] = React.useState(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you can switch this to a ref it doesn't need to be useState as its not used in any hooks you are just tracking a boolean state.

Copy link
Contributor Author

@ddoemonn ddoemonn Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can update it but there is something I didn't understand help me if I'm missing something. I put boolean inside of state because I wanted trigger a boolean value with using ref how can I achieve that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you jiust do yourRef.current = true or false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, I did the same but like I said we should trigger the component. You can try it in your local or I can commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting i was not expecting that based on looking at this code. You are just using a boolean flag. Changing the state is not used by any hook?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melloware you merged but there is prettier error what should I do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I can fix it. Next time run npm run format on the codebase it fixes it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will be the release date?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Luko248 that is up to PrimeTek I don't have any insight on release dates.

const metaData = {
props,
state: {
Expand Down Expand Up @@ -110,7 +111,7 @@ export const Tooltip = React.memo(

if (event === 'both') {
showEvents = ['focus', 'mouseenter'];
hideEvents = ['blur', 'mouseleave'];
hideEvents = multipleFocusEvents ? ['blur'] : ['mouseleave', 'blur'];
}
}

Expand Down Expand Up @@ -172,6 +173,8 @@ export const Tooltip = React.memo(
};

const show = (e) => {
if(e.type && e.type==="focus") setMultipleFocusEvents(true);

currentTargetRef.current = e.currentTarget;
const disabled = isDisabled(currentTargetRef.current);
const empty = isContentEmpty(isShowOnDisabled(currentTargetRef.current) && disabled ? currentTargetRef.current.firstChild : currentTargetRef.current);
Expand All @@ -198,6 +201,8 @@ export const Tooltip = React.memo(
};

const hide = (e) => {
if(e && e.type==="blur") setMultipleFocusEvents(false);

clearTimeouts();

if (visibleState) {
Expand Down
Loading