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(neon_talk): Improve message list constraints #1849

Merged
merged 2 commits into from
Apr 14, 2024

Conversation

provokateurin
Copy link
Member

Fixes #1740

This not only adds the padding so that the scroll bar never overlaps the message time, but also moves the constraint into the list itself so that the scrollbar is always at the most right position regardless of the list width and you can scroll everywhere.

I didn't find any way to use SliverConstrainedCrossAxis because it aligns everything to the left but the messages should be centered. Maybe @Leptopoda you know what would make this work.

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 28.63%. Comparing base (8f2f89d) to head (9017f74).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1849      +/-   ##
==========================================
- Coverage   28.63%   28.63%   -0.01%     
==========================================
  Files         197      197              
  Lines       67585    67586       +1     
==========================================
  Hits        19356    19356              
- Misses      48229    48230       +1     
Flag Coverage Δ *Carryforward flag
dynamite 44.18% <ø> (ø) Carriedforward from 8f2f89d
dynamite_end_to_end_test 61.38% <ø> (ø) Carriedforward from 8f2f89d
dynamite_runtime 82.65% <ø> (ø) Carriedforward from 8f2f89d
neon_dashboard 92.56% <ø> (ø)
neon_framework 37.61% <0.00%> (-0.02%) ⬇️
neon_talk 100.00% <ø> (ø)
nextcloud 25.76% <ø> (ø) Carriedforward from 8f2f89d

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
...n_framework/lib/src/models/app_implementation.dart 0.00% <0.00%> (ø)

@Leptopoda
Copy link
Member

I didn't find any way to use SliverConstrainedCrossAxis because it aligns everything to the left but the messages should be centered. Maybe @Leptopoda you know what would make this work.

I don't know of any way to make it work with a SliverConstrain
This boils down to the question of what you actually what to constrain. The chat view or the messages themselves?
If we ever want to implement a chat bubble UI (like Signal/WhatsApp use) you do not actually want to constrain the chat list as the bubbles should be alligned to the edges of the list/screen.
In this case I'd rather just put the old Center/ConstrainedBox inside the TalkMessage widget.

@provokateurin
Copy link
Member Author

Only the messages should be constrained. Right now I just constrained the whole sliver, but I guess constraining the individual messages is also possible. Then the padding can be applied to the whole list and it's separate from the width constraint.

@provokateurin
Copy link
Member Author

I couldn't find any other way to make it work, using ConstrainedBox on the TalkMesssage widget has no effect (but I don't understand why).

@Leptopoda
Copy link
Member

talk_constraints.patch.txt

This ontop of this branch works for me. Or are you trying to achieve something different?

…o getter

Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin provokateurin force-pushed the fix/neon_talk/message-list-constraints branch from 7ca02c0 to 9017f74 Compare April 14, 2024 11:51
@provokateurin
Copy link
Member Author

I almost had the same patch as you sent, but it didn't work. Now it did 🤷‍♀️

@provokateurin provokateurin merged commit 4fa25cd into main Apr 14, 2024
8 of 10 checks passed
@provokateurin provokateurin deleted the fix/neon_talk/message-list-constraints branch April 14, 2024 12:19
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.

Talk room page scrollbar overlaps message time
2 participants