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

fix(styles): prevent typing indicator overlay #9826

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

dijeth
Copy link
Contributor

@dijeth dijeth commented Jun 21, 2023

☑️ Resolves

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🚧 Tasks

  • Behaviour check

🏁 Checklist

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Hello and thanks for contributing to us!
It's a nice work, but i have found a couple of places, which could be improved. Please see the comments below:

And in addition to that (saw from your screenshot): could the position of "Scroll to bottom" button be corrected, like on the left screenshot?

src/components/NewMessage/NewMessage.vue Outdated Show resolved Hide resolved
src/components/NewMessage/NewMessage.vue Outdated Show resolved Hide resolved
src/components/NewMessage/NewMessage.vue Outdated Show resolved Hide resolved
src/components/NewMessage/NewMessageTypingIndicator.vue Outdated Show resolved Hide resolved
src/components/NewMessage/NewMessageTypingIndicator.vue Outdated Show resolved Hide resolved
@dijeth
Copy link
Contributor Author

dijeth commented Jun 23, 2023

Thank you for the comments!
I've done all of them and added a sticky position to scroll-to-bottom button and now its position depends on the height of "new message" block.

image

src/components/NewMessage/NewMessage.vue Outdated Show resolved Hide resolved
src/components/NewMessage/NewMessage.vue Show resolved Hide resolved
src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
@dijeth
Copy link
Contributor Author

dijeth commented Jun 23, 2023

If we hardcode the position of the button, it may happen that button will overlap the text.
Also, the position of the button seems strange when no one is typing. Perhaps we could set the bottom value depending on the typing status.

image

Wouldn't it be better if we hid the button using opacity? This way the scrolling will not jump and the button position will be adaptive. But we will need to remove the extra space under the text...

scroll-button

@Antreesy
Copy link
Contributor

But we will need to remove the extra space under the text...

That's the side effect of position: sticky I want to avoid

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Almost there! I have some thoughts on how to make scroll-to-bottom button work, but it requires a lot of code movements. How about we squash your commits into one and merge, and work on button in separate PR?

src/components/NewMessage/NewMessage.vue Outdated Show resolved Hide resolved
Signed-off-by: Dmtriy Orlov <d.orlov777@gmail.com>
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Position of "Scroll to bottom" goes to follow-up, as it needs more than a style refactoring.
The indicator itself is looking great now, thanks again!

@Antreesy Antreesy merged commit 0134719 into nextcloud:master Jun 26, 2023
@Antreesy
Copy link
Contributor

/backport to stable27

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

Successfully merging this pull request may close these issues.

Typing indicator is too close to the message input
3 participants