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.

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

It's possible to hover over the very edge of menu items in settings and you trigger the hover but you can’t click it #395

Closed
krschacht opened this issue Jun 8, 2024 · 7 comments · Fixed by #543
Labels
good first issue Good for newcomers
Milestone

Comments

@krschacht
Copy link
Contributor

krschacht commented Jun 8, 2024

This is the menu item that I mean. Try putting your cursor on the very edge of it and clicking:
image

This is almost surely because the hover CSS effect is being applied to the container of the a href, and there is likely some internal padding or margin on that. Maybe we can fix this by simply moving the hover effect from the outer container to the a href. Although, that may change the visual size of the hover effect which we don’t want. Probably the better fix is to identify whatever margin or padding is on the container and see if we can change margin > padding or padding > margin, not sure which way we’d need to tweak it. Or maybe we can move that padding/margin from the container into the a href interior item, which would increase it’s clickable size.

@krschacht krschacht added this to the 0.7 milestone Jun 8, 2024
@krschacht krschacht changed the title It's possible to hover over settings menu item but it's not clickable It's possible to hover over the very edge of menu items in settings and you trigger the hover but you can’t click it Jun 11, 2024
@krschacht krschacht added the good first issue Good for newcomers label Jun 11, 2024
@krschacht krschacht modified the milestones: 0.7, 0.8 Jun 11, 2024
@voodoo
Copy link
Contributor

voodoo commented Nov 12, 2024

https://github.com/voodoo/open-hostedgpt/commit/5079f6f8d7de27da23188303026dfe6ee884add7

  • Except for the fact it was done in one shot with AI, this commit is unremarkable
  • I'd been fussing around with the p and m for an hour or so
  • Then I thought, why don't I ask AI? Genius!
  • Works the way it should
  • Shorter

First time. I've been doing this AI stuff for a bit. That kinda blew me a way.

Tries to get a screenshot of the cursor to show you ... but ... I deployed that in no seconds flat - well done sir :)

https://hostedgpt-wyse.onrender.com/settings/assistants/2/edit

Is it GOOD? Dunno. Prolly not. It checks a lot of boxes.

gpt1.mov

Should I try to get a pull together? (What kind of test would you use - some sort of screen shot, I guess)

@krschacht
Copy link
Contributor Author

Ohhh, I see what the issue is and I see how you fixed it! Actually, this is an issue both within the settings and also outside the settings (the normal list of assistants). Your fix works, but I'd like to fix it in a slightly different way just to avoid introducing another inner element. You did the hard part of identifying it and figuring out the root cause, thanks!! I'll tee that up right now that fixes it a bit differently.

@krschacht
Copy link
Contributor Author

Done! I just merged it in.

If you're curious, the root of the issue is that the visual hover background was not on the a link itself, instead it was on the div that surrounded that link. But that div surrounding it had padding and that's what was not clickable. I just moved the padding from the surrounding div down into the a link itself so that the actual padding is part of the clickable element.

@voodoo
Copy link
Contributor

voodoo commented Nov 13, 2024

Makes sense. I fiddled around with the div's p and m - and even h if i recall.

That ai part was the remarkable part.

I'll try to find another issue.

Great project. Cheers.

@krschacht
Copy link
Contributor Author

Oh if you want another one! Out 3 words in your name (dr Joe smith) or (Mary Joe sue) and now your avatar has 3 initials and it overflows the circle. It needs to never be more than two initials.

@voodoo
Copy link
Contributor

voodoo commented Nov 13, 2024

Do I need to start an issue? (Great, I need the practice) If I'm not missing the forest for the trees ...

def user_initials(name, limit: 2)
name.split.first(limit).map(&:first).join.upcase
end

I added the .cursorrules but I need to take it out.

https://github.com/voodoo/open-hostedgpt/commit/afaaed46a4d63fdadf804b0ac3c594063cf7499c

"Dr Joe Smith" -> "DJ"
"Mary Joe Sue" -> "MJ"
"John Doe" -> "JD"
"John" -> "J"
This ensures we never get more than 2 initials, keeping the avatar display clean and preventing overflow issues in the circular container.

Something along those lines?

@krschacht
Copy link
Contributor Author

You don’t have to create an issue. But do create a PR.

You should create a method that takes user.name.initials as its argument. We are already properly extracting initials, we just want fewer of them. So make a helper like you’ve done that takes in all initials and returns a max of two. If there are more than two, keep the first and last initial and drop any in the middle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants