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

client: fix focusing input when clicking chat container #364

Merged
merged 1 commit into from Jun 19, 2016

Conversation

williamboman
Copy link
Member

@williamboman williamboman commented May 30, 2016

Not sure why it delegates events to the .messages container.

@williamboman williamboman changed the title client: fix focusing input when clicking chat box client: fix focusing input when clicking chat container May 30, 2016
@maxpoulin64
Copy link
Member

There's no need to change this. There's a reason it delegates for .messages: that's the only place that makes sense to focus the input on when you click. There's no point focusing the input when you click on the menu, or when you use the users list. This also breaks the search box in the users list, as you forcibly put the focus back on the chat input, so you can't search anymore. 👎

@williamboman
Copy link
Member Author

Oh I didn't realise chat is the #chat wrapper. What about #chat .chat? If there aren't enough messages the .messages container won't cover enough area vertically to capture all click events in the chat area.

@maxpoulin64
Copy link
Member

Just add a min-height: 100%; to #chat .chat .messages, that will make it cover it all and fix the issue.

@williamboman
Copy link
Member Author

Just add a min-height: 100%; to #chat .chat .messages, that will make it cover it all and fix the issue.

Idk

@maxpoulin64
Copy link
Member

Hmm, that worked fine for me on Chrome. Another option would be to check that the event's e.target was either .chat or .messages and do nothing else otherwise. We could also drop the events in the other panels, but it's very kludgy and error-prone if we end up adding another panel so I'd really prefer the other option.

@xPaw
Copy link
Member

xPaw commented Jun 3, 2016

Would it work if you delegated on .chat?

@zakness
Copy link

zakness commented Jun 3, 2016

This bug is one of my biggest frustrations, thanks for looking into it!

The stretching issue with min-height is due to display: table-* rules on .messages and its children. Have you tried using flexbox for layout (assuming you dont care about old browsers)? In any case, simply removing display: table from #chat .messages seems to do the trick while still preserving the layout (at least on Chrome). Then add the min-height to fix the click-focus:

#chat .messages {
  display: table; /* remove this */
  min-height: 100%; /* add this */
}

@xPaw
Copy link
Member

xPaw commented Jun 5, 2016

👍 .chat delegate fixes the issue. @maxpoulin64 confirm?

@xPaw xPaw added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. second review needed labels Jun 5, 2016
@astorije
Copy link
Member

👍 and merging.

@astorije astorije merged commit 500c6e2 into thelounge:master Jun 19, 2016
@xPaw xPaw added this to the 2.0.0 milestone Jun 19, 2016
@williamboman williamboman deleted the fix/chat-input-focus branch June 21, 2016 09:52
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
client: fix focusing input when clicking chat container
@xPaw xPaw unassigned astorije and xPaw Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants