Skip to content

feat: add tooltip element to the builder#246

Open
jz5426 wants to merge 24 commits into
IBM:mainfrom
jz5426:dev.tooltip
Open

feat: add tooltip element to the builder#246
jz5426 wants to merge 24 commits into
IBM:mainfrom
jz5426:dev.tooltip

Conversation

@jz5426

@jz5426 jz5426 commented May 18, 2023

Copy link
Copy Markdown
Contributor

closes #132

@netlify

netlify Bot commented May 18, 2023

Copy link
Copy Markdown

👷 Deploy request for carbon-components-builder pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 2462361

@jz5426
jz5426 marked this pull request as ready for review May 19, 2023 12:06
@jz5426 jz5426 changed the title fea: add tooltip elements to the builder feat: add tooltip elements to the builder May 19, 2023
@jz5426 jz5426 changed the title feat: add tooltip elements to the builder feat: add tooltip element to the builder May 19, 2023

@zvonimirfras zvonimirfras left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use the icon from here

@jz5426
jz5426 force-pushed the dev.tooltip branch 2 times, most recently from 794108b to da47417 Compare June 8, 2023 14:06
@jz5426
jz5426 requested a review from zvonimirfras June 8, 2023 14:20
Comment thread src/sdk/src/fragment-components/a-tooltip.tsx Outdated
<AComponent
componentObj={componentObj}
rejectDrop={true}
// className={css`position: relative; display: inline-flex`}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

select={select}
remove={remove}
selected={selected}>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

?

Also, if there's no children, it should be a self-closing tag, no? <Tooltip ... />

Comment on lines +125 to +126
placement: 'top',
alignment: 'center',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

are these default values? if yes, we don't (shouldn't) need to specify them, if bloats the json.

outputs: () => '',
imports: ['TooltipModule'],
code: ({ json }) => {
return `<div class="bx--tooltip__label">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can be inline (without the return statement)?

react: {
imports: ['Tooltip'],
code: ({ json }) => {
return `<Tooltip

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can be inline (without the return statement)?

return <Tooltip
description={state.description}
direction={state.placement ? state.placement : 'top'}
align={state.alignment ? state.alignment : 'center'}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

indentation

}
return <Tooltip
description={state.description}
direction={state.placement ? state.placement : 'top'}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is it "placement" if the property it's used for is "direction"?
Also, how will this change in v11?

Max You added 6 commits June 9, 2023 17:42
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Max You Max You
svg
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Max You Max You
svg
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
jz5426 and others added 12 commits October 12, 2023 13:43
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <max.you@outlook.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
@jz5426

jz5426 commented Apr 25, 2024

Copy link
Copy Markdown
Contributor Author

Verified that the component features and export works.

Max You added 2 commits June 3, 2024 17:21
Signed-off-by: Max You <JianzhongMax.You@ibm.com>

@zvonimirfras zvonimirfras left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

input and output names should be camelCase

Comment thread player/react/src/lib/components/ui-definition-tooltip.tsx Outdated
Comment thread player/react/src/lib/components/ui-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
className={css`${styleObjectToString(componentObj.style)}`}
definition={componentObj.definition}
align={componentObj.alignment}>
{componentObj.description}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In both frameworks description is a child, so why don't we do that here too? Meaning description wouldn't be part of the componentObj.description - instead we just let items render.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we stick with support only text only for now

Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
Comment thread sdk/react/src/lib/fragment-components/a-definition-tooltip.tsx Outdated
jz5426 and others added 3 commits July 22, 2024 16:57
Co-authored-by: Zvonimir Fras <zvonimir.fras@gmail.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Signed-off-by: Max You <JianzhongMax.You@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: QA/Review

Development

Successfully merging this pull request may close these issues.

Add Tooltip

2 participants