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

Hotels: SummaryRow should use dynamic width to truncate text #1609

Merged
merged 1 commit into from May 7, 2019

Conversation

Projects
None yet
2 participants
@RobinCsl
Copy link
Collaborator

commented May 6, 2019

This fixes #1607

The issue was that using the withDimension HOC meant using the dimensions of the device. On Tablet view, the RoomSummary component is displayed only on half the screen width while SummaryRow assumed there is the full width of the device to render the text. This caused the row to go out of the container if the name of the hotel room was too long on Tablet.

The SummaryRow component was refactored to use two onLayout event listeners so that both the width of the container of the row and the width of the price tag are computed, and the max width for text can be deduced from that.

Hotels: SummaryRow should use dynamic width to truncate text
This fixes #1607

The issue was that using the `withDimension` HOC meant using the dimensions of the device. On Tablet view, the RoomSummary component is displayed only on half the screen width while SummaryRow assumed there is the full width of the device to render the text. This caused the row to go out of the container if the name of the hotel room was too long on Tablet.

The SummaryRow component was refactored to use two onLayout event listeners so that both the width of the container of the row and the width of the price tag are computed, and the max width for text can be deduced from that.

@RobinCsl RobinCsl requested a review from mrtnzlml May 6, 2019

@RobinCsl RobinCsl requested a review from tbergq as a code owner May 6, 2019

const maxWidth =
containerWidth != null && priceWidth != null
? containerWidth - priceWidth - SPACING
: 0;

This comment has been minimized.

Copy link
@mrtnzlml

mrtnzlml May 6, 2019

Member

Does it behave correctly when the max width is zero?

This comment has been minimized.

Copy link
@RobinCsl

RobinCsl May 7, 2019

Author Collaborator

Setting the max width to zero just displays the price on the right hand-side, and the text is obviously not showing. That's an acceptable behaviour for me (especially when it should not happen after the Views rendered and executed their onLayout).

Screenshot 2019-05-07 at 09 59 24
Screenshot 2019-05-07 at 10 01 00

@RobinCsl RobinCsl merged commit 0d4fb94 into master May 7, 2019

1 check passed

ci/gitlab/roomsummary-refactor Pipeline passed on GitLab
Details

@RobinCsl RobinCsl deleted the roomsummary-refactor branch May 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.