-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: add tooltip element to the builder #246
base: main
Are you sure you want to change the base?
Conversation
👷 Deploy request for carbon-components-builder pending review.Visit the deploys page to approve it
|
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.
Use the icon from here
794108b
to
da47417
Compare
<AComponent | ||
componentObj={componentObj} | ||
rejectDrop={true} | ||
// className={css`position: relative; display: inline-flex`} |
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.
?
select={select} | ||
remove={remove} | ||
selected={selected}> | ||
|
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.
?
Also, if there's no children, it should be a self-closing tag, no? <Tooltip ... />
placement: 'top', | ||
alignment: 'center', |
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.
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"> |
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.
can be inline (without the return statement)?
react: { | ||
imports: ['Tooltip'], | ||
code: ({ json }) => { | ||
return `<Tooltip |
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.
can be inline (without the return statement)?
return <Tooltip | ||
description={state.description} | ||
direction={state.placement ? state.placement : 'top'} | ||
align={state.alignment ? state.alignment : 'center'} |
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.
indentation
} | ||
return <Tooltip | ||
description={state.description} | ||
direction={state.placement ? state.placement : 'top'} |
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.
Why is it "placement" if the property it's used for is "direction"?
Also, how will this change in v11?
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
Verified that the component features and export works. |
Signed-off-by: Max You <[email protected]>
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.
input and output names should be camelCase
className={css`${styleObjectToString(componentObj.style)}`} | ||
definition={componentObj.definition} | ||
align={componentObj.alignment}> | ||
{componentObj.description} |
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.
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.
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.
we stick with support only text only for now
Co-authored-by: Zvonimir Fras <[email protected]>
Signed-off-by: Max You <[email protected]>
Signed-off-by: Max You <[email protected]>
closes #132