-
Couldn't load subscription status.
- Fork 34
DONT-REVIEW-TEST #3039
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
base: alpha
Are you sure you want to change the base?
DONT-REVIEW-TEST #3039
Conversation
23cb3de to
95ff993
Compare
069e025 to
0ab5690
Compare
|
-Added React playground code, and rebased with alpha |
| width: width.includes("%") ? width : `min(${width}, 100%)`, | ||
| }); | ||
| const finalWidth = width.includes("%") ? width : `min(${width}, 100%)`; | ||
| const cssProps: Record<string, string> = { width: finalWidth }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a slight issue. width and max-width and min-width are all being applied simultaneously in different spots. You're applying width, and max-width here in this onMount.
widthis also applied on line 172 inside the divrootmax-widthis also applied on line 230 inside the CSS for.rootmin-widthandmax-widthare also being applied on lines 283 and 284 inside the CSS fortextareamin-widthandmax-widthare also being applied on lines 329, 330, and 336, 337 inside the CSS for mobile and for not-mobile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be because of above, I'm not sure. But TextArea width set to a percentage value doesn't work as expected.
| // Public | ||
| export let content = ""; | ||
| export let testid: string = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tooltip using maxWidth with maxWidth set to a percentage doesn't display accurately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ch also doesn't work as expected:
maxWidth set to 10ch:

Using this code:
<goab-block direction="column" gap="m">
<goab-text tag="h2">Tooltip examples</goab-text>
<goab-grid minChildWidth="260px" gap="m">
<goab-block
*ngFor="let example of tooltipExamples; index as index"
direction="column"
gap="s"
>
<goab-text tag="h3">{{ example.label }}</goab-text>
<goab-text tag="p">{{ example.description }}</goab-text>
<goab-tooltip
[content]="example.content"
[maxWidth]="example.maxWidth"
[style.width]="example.width"
>
<goab-icon type="information-circle" size="3"></goab-icon>
</goab-tooltip>
</goab-block>
</goab-grid>
</goab-block>
readonly tooltipExamples: TooltipExample[] = [
{
label: "Default tooltip",
description: "Relies on the default tooltip sizing for comparison.",
content:
"This tooltip keeps the default sizing so longer text can expand without additional constraints.",
},
{
label: "Style width 500px, maxWidth 200px",
description: "Applies an inline width while capping the tooltip content at 200px.",
width: "500px",
maxWidth: "200px",
content:
"Inline width is set to 500px but the tooltip maxWidth is restricted to 200px to demonstrate clamping.",
},
{
label: "Max width 300px",
description: "Limits the tooltip content to 300px without changing width.",
maxWidth: "100ch",
content:
"The tooltip content should wrap within 300px, demonstrating the maxWidth attribute in pixels.",
},
{
label: "Max width 50%",
description: "Uses a responsive maxWidth to scale with the parent container.",
width: "500px",
maxWidth: "100%",
content:
"This tooltip uses a percentage based maxWidth so it will shrink or grow with its container width.",
},
];| export let value: string | undefined = ""; | ||
| export let filterable: string = "false"; | ||
| export let leadingicon: GoAIconType | null = null; | ||
| export let maxheight: string = "276px"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated and angular playground code updated also.
| describe("Dropdown Component", () => { | ||
| const noop = () => { | ||
| // noop | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all the browser tests, you're only testing basic px measurements. You should also have tests using % and ch to make sure those work as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated.
| const spaceLeft = rootRect.left; | ||
| const spaceRight = window.innerWidth - rootRect.right; | ||
| const calculatedMaxWidth = maxwidth ? parseFloat(maxwidth) : 400; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work in the case that a user specifies a non-px value. Since the calculation is required to determine which side the tooltip will appear on, for now let's restrict them to using px values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated.
| export let rows: number = 3; | ||
| export let testid: string = ""; | ||
| export let width: string = "60ch"; | ||
| export let maxwidth: string = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To prevent the current width default property from conflicting with the maxwidth, change to the following
export let width: string = "100%"
export let maxwidth: string = "60ch"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently when setting the maxWidth it behaves like width, in that if the screen is resized the component stays set at the max width (this is true for both the textarea and dropdown), but it would be assumed by devs that the component would either be at 100% or the maxwidth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR updated.
5a9a809 to
c2d2b54
Compare



Before (the change)
After (the change)
Make sure that you've checked the boxes below before you submit the PR
Steps needed to test
Playground Code
Angular
angular-issue-2054.html.txt
angular-issue-2054.ts.txt
React