Skip to content

fix bug in show-hints inset #7129

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.

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

Closed
wants to merge 1 commit into from

Conversation

alirrah
Copy link

@alirrah alirrah commented Jun 24, 2025

When the show-hints box does not have space to be displayed below, the box will be displayed in the wrong place, which is also reported in this issue. I fixed this issue by making some changes, which you can see in the videos below.

The reason I added marginBottom is so that I can give the box some space from the bottom (I didn't want the box to stick to the bottom of the page).

before:

vokoscreenNG-2025-06-24_09-52-21.webm

after:

vokoscreenNG-2025-06-24_09-49-06.webm

@marijnh
Copy link
Member

marijnh commented Jun 24, 2025

Do you have a minimal HTML page that shows the current issue happening? My response to the issue you linked suggests I haven't been able reproduce the issue.

@alirrah alirrah force-pushed the hints-inset-position branch 2 times, most recently from aca92ea to 0a88b8c Compare June 30, 2025 06:15
@alirrah alirrah force-pushed the hints-inset-position branch from 0a88b8c to 75f56c3 Compare June 30, 2025 06:18
@alirrah
Copy link
Author

alirrah commented Jun 30, 2025

You can see the error at the link below:
https://codesandbox.io/p/sandbox/wcndgs

As can be seen in line 272, the value of offsetTop should be subtracted from top, whereas in line 294 it has been added.

@marijnh marijnh closed this in 9f1450d Aug 10, 2025
@marijnh
Copy link
Member

marijnh commented Aug 10, 2025

I can confirm that that plus should be a minus. I've corrected that in attached patch. I don't think adding an option like marginBottom is a good idea—firstly, it suggests we'd also need marginTop, marginLeft, and marginRight, and secondly, I don't think your implementation, as it stands, properly implements a bottom margin (just blindly moving the tooltip up will cause all the other computations to work with the wrong position). As such, I don't intend to merge that part, and suggest you work with something like transparent borders around the tooltip to get an effect like this.

@alirrah alirrah deleted the hints-inset-position branch August 10, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants