Skip to content

Refactor compose-box layout with error banner #861

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

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Aug 2, 2024

This is a followup to #816, as foreshadowed at #816 (comment) .

This lets _ComposeBoxLayout stay focused on the complexity of how the text inputs, the editing buttons, and the send button relate to each other. In the case where an error banner is present, this also lets us skip in the build method some logic to compute things that won't end up getting used.

@gnprice gnprice requested review from chrisbobbe and sm-sayedi August 2, 2024 20:10
@chrisbobbe
Copy link
Collaborator

Thanks! LGTM, and I see this is also marked for @sm-sayedi's review.

Copy link
Collaborator

@sm-sayedi sm-sayedi left a comment

Choose a reason for hiding this comment

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

Thanks @gnprice for the refactor. _ComposeBoxLayout was really getting kind of cluttered! 😀 LGTM now!

… caller

This lets _ComposeBoxLayout stay focused on the complexity of how
the text inputs, the editing buttons, and the send button relate
to each other.

In the case where an error banner is present, this also lets us skip
in the build method some logic to compute things that won't end up
getting used.
This way the caller doesn't have to do so.

This change is NFC because in the non-error-banner case, where the
caller is _ComposeBoxLayout, the child already fills the available
horizontal space because of the Expanded inside the Row.
@gnprice
Copy link
Member Author

gnprice commented Aug 13, 2024

Thanks for the reviews! Merging.

@gnprice gnprice force-pushed the pr-compose-box-layout branch from 2523f60 to 96b4f08 Compare August 13, 2024 04:29
@gnprice gnprice merged commit 96b4f08 into zulip:main Aug 13, 2024
1 check passed
@gnprice gnprice deleted the pr-compose-box-layout branch August 13, 2024 04:32
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.

3 participants