stream-chat-flutter icon indicating copy to clipboard operation
stream-chat-flutter copied to clipboard

Published 20 hours ago • verified publisher icon GetStream

feat(ui): sendingIndicatorBuilder

Open geweald opened this issue 3 years ago • 1 comments

Submit a pull request

CLA

  • [x] I have signed the Stream CLA (required).
  • [ ] The code changes follow best practices
  • [ ] Code changes are tested (add some information if not applicable)

Description of the pull request

Similar to usernameBuilder and others it allows customizing sending indicator in BottomRow without touching the whole BottomRow of the message

geweald avatar Oct 10 '22 13:10 geweald

Codecov Report

Base: 58.03% // Head: 58.02% // Decreases project coverage by -0.00% :warning:

Coverage data is based on head (5f96a9e) compared to base (144d517). Patch coverage: 20.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #1349      +/-   ##
===========================================
- Coverage    58.03%   58.02%   -0.01%     
===========================================
  Files          286      286              
  Lines        17082    17086       +4     
===========================================
+ Hits          9913     9915       +2     
- Misses        7169     7171       +2
Impacted Files Coverage Δ
...hat_flutter/lib/src/message_widget/bottom_row.dart 45.83% <0.00%> (-0.65%) :arrow_down:
...flutter/lib/src/message_widget/message_widget.dart 26.84% <50.00%> (+0.12%) :arrow_up:
...lib/src/message_widget/message_widget_content.dart 57.39% <100.00%> (+0.25%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Oct 10 '22 14:10 codecov[bot]

I don't believe this is the best solution for this kind of use case.

Based on what we're doing in other places in the codebase I would:

  • add a sendingIndicatorBuilder to the BottomRow widget
  • add a copyWith method to the BottomRow widget, so that we can modify an instance easily
  • add the default BottomRow widget implementation as the third parameter in the bottomRowBuilder callback

what do you think

cc @xsahil03x

imtoori avatar Oct 24 '22 08:10 imtoori

Yes, I agree with @imtoori, as then there will be multiple builders for the whole bottom row elements.

xsahil03x avatar Oct 24 '22 08:10 xsahil03x

Thanks guys, it makes sense too, I based it on usernameBuilder which is a part of a BottomRow, so it'd be good to refactor this too, tho it'd introduce a breaking change ;/ so yea, I'll adjust to your suggestions :)

geweald avatar Oct 26 '22 09:10 geweald

@geweald let me know once you get to that, we can resume the conversation.

petyosi avatar Nov 21 '22 14:11 petyosi

@petyosi created a new PR, not sure tho if I should remove all the builders now from the message widget that are later on used in the bottom row? As people can now add them with copyWith

geweald avatar Dec 06 '22 12:12 geweald

Hey @geweald, you can mark them deprecated with some info about the new implementation.

xsahil03x avatar Dec 06 '22 14:12 xsahil03x