msyavuz commented on code in PR #32890:
URL: https://github.com/apache/superset/pull/32890#discussion_r2025570785
##########
superset-frontend/src/components/IconButton/index.tsx:
##########
@@ -16,129 +16,94 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { styled } from '@superset-ui/core';
-import Button, { ButtonProps as AntdButtonProps } from 'src/components/Button';
+// eslint-disable-next-line
+import { Typography } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import Card, { CardProps } from 'src/components/Card';
import Icons from 'src/components/Icons';
-import LinesEllipsis from 'react-lines-ellipsis';
+import { SupersetTheme } from '@superset-ui/core';
-export interface IconButtonProps extends AntdButtonProps {
+export interface IconButtonProps extends CardProps {
buttonText: string;
icon: string;
altText?: string;
}
-const StyledButton = styled(Button)`
- height: auto;
- display: flex;
- flex-direction: column;
- padding: 0;
-`;
-
-const StyledImage = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px;
- height: ${({ theme }) => theme.gridUnit * 18}px;
- margin: ${({ theme }) => theme.gridUnit * 3}px 0;
-
- .default-db-icon {
- font-size: 36px;
- color: ${({ theme }) => theme.colors.grayscale.base};
- margin-right: 0;
- span:first-of-type {
- margin-right: 0;
- }
- }
-
- &:first-of-type {
- margin-right: 0;
- }
-
- img {
- width: ${({ theme }) => theme.gridUnit * 10}px;
- height: ${({ theme }) => theme.gridUnit * 10}px;
- margin: 0;
- &:first-of-type {
- margin-right: 0;
- }
- }
- svg {
- &:first-of-type {
- margin-right: 0;
+const IconButton: React.FC<IconButtonProps> = ({
+ buttonText,
+ icon,
+ altText,
+ ...cardProps
+}) => {
+ const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ if (cardProps.onClick) {
+ cardProps.onClick(e as any);
+ }
+ if (e.key === ' ') {
+ e.preventDefault();
+ }
}
- }
-`;
-
-const StyledInner = styled.div`
- max-height: calc(1.5em * 2);
- white-space: break-spaces;
-
- &:first-of-type {
- margin-right: 0;
- }
-
- .LinesEllipsis {
- &:first-of-type {
- margin-right: 0;
- }
- }
-`;
-
-const StyledBottom = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px 0;
- border-radius: 0 0 ${({ theme }) => theme.borderRadius}px
- ${({ theme }) => theme.borderRadius}px;
- background-color: ${({ theme }) => theme.colors.grayscale.light4};
- width: 100%;
- line-height: 1.5em;
- overflow: hidden;
- white-space: no-wrap;
- text-overflow: ellipsis;
-
- &:first-of-type {
- margin-right: 0;
- }
-`;
+ cardProps.onKeyDown?.(e);
+ };
-const IconButton = styled(
- ({ icon, altText, buttonText, ...props }: IconButtonProps) => (
- <StyledButton {...props}>
- <StyledImage>
- {icon && <img src={icon} alt={altText} />}
- {!icon && (
- <Icons.DatabaseOutlined
- className="default-db-icon"
- aria-label="default-icon"
- />
- )}
- </StyledImage>
+ const renderIcon = () => {
+ const iconContent = icon ? (
+ <img
+ src={icon}
+ alt={altText || buttonText}
+ css={(theme: SupersetTheme) => ({
+ width: '100%',
+ height: '120px',
+ objectFit: 'contain',
+ })}
+ />
+ ) : (
+ <div
+ css={(theme: SupersetTheme) => ({
Review Comment:
same here
##########
superset-frontend/src/components/IconButton/index.tsx:
##########
@@ -16,129 +16,94 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { styled } from '@superset-ui/core';
-import Button, { ButtonProps as AntdButtonProps } from 'src/components/Button';
+// eslint-disable-next-line
+import { Typography } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import Card, { CardProps } from 'src/components/Card';
import Icons from 'src/components/Icons';
-import LinesEllipsis from 'react-lines-ellipsis';
+import { SupersetTheme } from '@superset-ui/core';
-export interface IconButtonProps extends AntdButtonProps {
+export interface IconButtonProps extends CardProps {
buttonText: string;
icon: string;
altText?: string;
}
-const StyledButton = styled(Button)`
- height: auto;
- display: flex;
- flex-direction: column;
- padding: 0;
-`;
-
-const StyledImage = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px;
- height: ${({ theme }) => theme.gridUnit * 18}px;
- margin: ${({ theme }) => theme.gridUnit * 3}px 0;
-
- .default-db-icon {
- font-size: 36px;
- color: ${({ theme }) => theme.colors.grayscale.base};
- margin-right: 0;
- span:first-of-type {
- margin-right: 0;
- }
- }
-
- &:first-of-type {
- margin-right: 0;
- }
-
- img {
- width: ${({ theme }) => theme.gridUnit * 10}px;
- height: ${({ theme }) => theme.gridUnit * 10}px;
- margin: 0;
- &:first-of-type {
- margin-right: 0;
- }
- }
- svg {
- &:first-of-type {
- margin-right: 0;
+const IconButton: React.FC<IconButtonProps> = ({
+ buttonText,
+ icon,
+ altText,
+ ...cardProps
+}) => {
+ const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ if (cardProps.onClick) {
+ cardProps.onClick(e as any);
+ }
+ if (e.key === ' ') {
+ e.preventDefault();
+ }
}
- }
-`;
-
-const StyledInner = styled.div`
- max-height: calc(1.5em * 2);
- white-space: break-spaces;
-
- &:first-of-type {
- margin-right: 0;
- }
-
- .LinesEllipsis {
- &:first-of-type {
- margin-right: 0;
- }
- }
-`;
-
-const StyledBottom = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px 0;
- border-radius: 0 0 ${({ theme }) => theme.borderRadius}px
- ${({ theme }) => theme.borderRadius}px;
- background-color: ${({ theme }) => theme.colors.grayscale.light4};
- width: 100%;
- line-height: 1.5em;
- overflow: hidden;
- white-space: no-wrap;
- text-overflow: ellipsis;
-
- &:first-of-type {
- margin-right: 0;
- }
-`;
+ cardProps.onKeyDown?.(e);
+ };
-const IconButton = styled(
- ({ icon, altText, buttonText, ...props }: IconButtonProps) => (
- <StyledButton {...props}>
- <StyledImage>
- {icon && <img src={icon} alt={altText} />}
- {!icon && (
- <Icons.DatabaseOutlined
- className="default-db-icon"
- aria-label="default-icon"
- />
- )}
- </StyledImage>
+ const renderIcon = () => {
+ const iconContent = icon ? (
+ <img
+ src={icon}
+ alt={altText || buttonText}
+ css={(theme: SupersetTheme) => ({
Review Comment:
theme is not used here. You can do something like:
```
css={css`
width: '100%',
height: '120px',
objectFit: 'contain',
`}
```
css function is imported from `@superset-ui/core`
##########
superset-frontend/src/components/IconButton/IconButton.stories.tsx:
##########
@@ -16,42 +16,37 @@
* specific language governing permissions and limitations
* under the License.
*/
-import IconButton, { IconButtonProps } from '.';
+import { Meta, StoryObj } from '@storybook/react';
+import IconButton from 'src/components/IconButton';
-export default {
- title: 'IconButton',
+const meta: Meta<typeof IconButton> = {
+ title: 'Components/IconButton',
component: IconButton,
+ argTypes: {
+ onClick: { action: 'clicked' },
+ },
+ parameters: {
+ a11y: {
+ enabled: true,
+ },
+ },
};
-export const InteractiveIconButton = (args: IconButtonProps) => (
- <IconButton
- buttonText={args.buttonText}
- altText={args.altText}
- icon={args.icon}
- href={args.href}
- target={args.target}
- htmlType={args.htmlType}
- />
-);
+export default meta;
-InteractiveIconButton.args = {
- buttonText: 'This is the IconButton text',
- altText: 'This is an example of non-default alt text',
- href: 'https://preset.io/',
- target: '_blank',
-};
+type Story = StoryObj<typeof IconButton>;
-InteractiveIconButton.argTypes = {
- icon: {
- defaultValue: '/images/icons/sql.svg',
- control: {
- type: 'select',
- },
- options: [
- '/images/icons/sql.svg',
- '/images/icons/server.svg',
- '/images/icons/image.svg',
- 'Click to see example alt text',
- ],
+export const Default: Story = {
+ args: {
+ buttonText: 'Default IconButton',
+ altText: 'Default icon button alt text',
},
};
+
+export const CustomIcon: Story = {
+ args: {
+ buttonText: 'Custom icon IconButton',
+ altText: 'Custom icon button alt text',
+ icon: '/images/sqlite.png'
+ },
+};
Review Comment:
You will have some formatting issues here after the conflicts are resolved.
##########
superset-frontend/src/components/IconButton/IconButton.stories.tsx:
##########
@@ -16,42 +16,37 @@
* specific language governing permissions and limitations
* under the License.
*/
-import IconButton, { IconButtonProps } from '.';
+import { Meta, StoryObj } from '@storybook/react';
+import IconButton from 'src/components/IconButton';
Review Comment:
Should be named import here as well
##########
superset-frontend/src/components/IconButton/index.tsx:
##########
@@ -16,129 +16,94 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { styled } from '@superset-ui/core';
-import Button, { ButtonProps as AntdButtonProps } from 'src/components/Button';
+// eslint-disable-next-line
+import { Typography } from 'src/components';
+import { Tooltip } from 'src/components/Tooltip';
+import Card, { CardProps } from 'src/components/Card';
import Icons from 'src/components/Icons';
-import LinesEllipsis from 'react-lines-ellipsis';
+import { SupersetTheme } from '@superset-ui/core';
-export interface IconButtonProps extends AntdButtonProps {
+export interface IconButtonProps extends CardProps {
buttonText: string;
icon: string;
altText?: string;
}
-const StyledButton = styled(Button)`
- height: auto;
- display: flex;
- flex-direction: column;
- padding: 0;
-`;
-
-const StyledImage = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px;
- height: ${({ theme }) => theme.gridUnit * 18}px;
- margin: ${({ theme }) => theme.gridUnit * 3}px 0;
-
- .default-db-icon {
- font-size: 36px;
- color: ${({ theme }) => theme.colors.grayscale.base};
- margin-right: 0;
- span:first-of-type {
- margin-right: 0;
- }
- }
-
- &:first-of-type {
- margin-right: 0;
- }
-
- img {
- width: ${({ theme }) => theme.gridUnit * 10}px;
- height: ${({ theme }) => theme.gridUnit * 10}px;
- margin: 0;
- &:first-of-type {
- margin-right: 0;
- }
- }
- svg {
- &:first-of-type {
- margin-right: 0;
+const IconButton: React.FC<IconButtonProps> = ({
+ buttonText,
+ icon,
+ altText,
+ ...cardProps
+}) => {
+ const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ if (cardProps.onClick) {
+ cardProps.onClick(e as any);
+ }
+ if (e.key === ' ') {
+ e.preventDefault();
+ }
}
- }
-`;
-
-const StyledInner = styled.div`
- max-height: calc(1.5em * 2);
- white-space: break-spaces;
-
- &:first-of-type {
- margin-right: 0;
- }
-
- .LinesEllipsis {
- &:first-of-type {
- margin-right: 0;
- }
- }
-`;
-
-const StyledBottom = styled.div`
- padding: ${({ theme }) => theme.gridUnit * 4}px 0;
- border-radius: 0 0 ${({ theme }) => theme.borderRadius}px
- ${({ theme }) => theme.borderRadius}px;
- background-color: ${({ theme }) => theme.colors.grayscale.light4};
- width: 100%;
- line-height: 1.5em;
- overflow: hidden;
- white-space: no-wrap;
- text-overflow: ellipsis;
-
- &:first-of-type {
- margin-right: 0;
- }
-`;
+ cardProps.onKeyDown?.(e);
+ };
-const IconButton = styled(
- ({ icon, altText, buttonText, ...props }: IconButtonProps) => (
- <StyledButton {...props}>
- <StyledImage>
- {icon && <img src={icon} alt={altText} />}
- {!icon && (
- <Icons.DatabaseOutlined
- className="default-db-icon"
- aria-label="default-icon"
- />
- )}
- </StyledImage>
+ const renderIcon = () => {
+ const iconContent = icon ? (
+ <img
+ src={icon}
+ alt={altText || buttonText}
+ css={(theme: SupersetTheme) => ({
+ width: '100%',
+ height: '120px',
+ objectFit: 'contain',
+ })}
+ />
+ ) : (
+ <div
+ css={(theme: SupersetTheme) => ({
+ display: 'flex',
+ alignContent: 'center',
+ alignItems: 'center',
+ height: '120px',
+ })}
+ >
+ <Icons.DatabaseOutlined
+ css={(theme: SupersetTheme) => ({
Review Comment:
and here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
