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(v2): fix overflow in workspace row #4069

Merged
merged 4 commits into from
Jul 5, 2022

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Jul 1, 2022

Problem

Extremely long form titles were resulting in incorrect overflow of the page. This PR fixes that bug.
Also replace deprecated usage of isTruncated prop and replace them with the noOfLine prop.

Solution

Breaking Changes

  • No - this PR is backwards compatible

Improvements:

  • ref: replace deprecated usage of isTruncated with noOfLines

Bug Fixes:

  • fix: correctly truncate workspace form row on long title

Before & After Screenshots

BEFORE:

Screenshot 2022-07-01 at 11 20 20 AM
AFTER:

Screenshot 2022-07-01 at 11 20 13 AM

@karrui karrui requested review from justynoh and wanlingt July 1, 2022 03:21
@@ -13,7 +13,7 @@ export const CollaboratorText: FC = ({ children }) => {
<Text
textStyle={{ base: 'subhead-1', md: 'body-2' }}
color={{ base: 'secondary.700', md: 'secondary.500' }}
isTruncated
noOfLines={{ base: 0, md: 1 }}
Copy link
Contributor

@wanlingt wanlingt Jul 1, 2022

Choose a reason for hiding this comment

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

If this is truncated, is there any other way to see the full emails of existing collaborators?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. added a title; hovering the row would now display the full email.

@@ -32,7 +32,7 @@ export const FieldLogicBadge = ({ field }: FieldLogicBadgeProps) => {
<Icon as={fieldMeta.icon} fontSize="1rem" />
</Tooltip>
{field.questionNumber ? <Text>{field.questionNumber}.</Text> : null}
<Text isTruncated>{field.title}</Text>
<Text noOfLines={1}>{field.title}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we truncate the field name here? feels like it does not need to be truncated right haha

the current behavior of dropdown logic does not truncate the option selected, as shown below

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no clue, preserving current behavior. can always fix in a new issue.

@@ -32,7 +32,7 @@ export const FieldLogicBadge = ({ field }: FieldLogicBadgeProps) => {
<Icon as={fieldMeta.icon} fontSize="1rem" />
</Tooltip>
{field.questionNumber ? <Text>{field.questionNumber}.</Text> : null}
<Text isTruncated>{field.title}</Text>
<Text noOfLines={1}>{field.title}</Text>
Copy link
Contributor

@justynoh justynoh Jul 1, 2022

Choose a reason for hiding this comment

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

unrelated to this PR: should fix delete box overlapping with long field title, regardless of truncated or not. For reference:

truncated
image

not truncated
image

edit: created issue #4076

@karrui karrui requested review from wanlingt and justynoh and removed request for justynoh July 5, 2022 03:03
Copy link
Contributor

@wanlingt wanlingt left a comment

Choose a reason for hiding this comment

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

lgtm!

@karrui karrui merged commit a81d419 into form-v2/develop Jul 5, 2022
@karrui karrui deleted the form-v2/fix-overflow-in-workspace-row branch July 5, 2022 03:37
@justynoh justynoh mentioned this pull request Oct 5, 2022
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