Skip to content

Conversation

Zxun2
Copy link
Contributor

@Zxun2 Zxun2 commented Nov 20, 2023

Changes

  • Updated existing atomic components to use Jupyterlab UI Toolkit

@Zxun2 Zxun2 marked this pull request as draft November 20, 2023 16:13
Copy link

Binder 👈 Launch a Binder on branch Zxun2/jupyterlab-git/zxun2/ui-toolkit

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Hey @Zxun2

Thanks for working on this. I see that you are based on the main branch prior to the migration to JupyterLab 4. Would you mind migrating to the main branch latest commit?

@Zxun2 Zxun2 marked this pull request as ready for review November 29, 2023 07:09
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @Zxun2

The action buttons are too big by default. I got something more acceptable by setting the following CSS property in actionButtonStyle (src/style/ActionButtonStyle.ts):

  // @ts-expect-error unknown prop
  '--density': -1,

Comment on lines +43 to +53
<>
<Tooltip anchor={title}>{title}</Tooltip>
<Button
id={title}
className={classes(actionButtonStyle, className)}
disabled={disabled}
onClick={onClick}
>
<icon.react elementPosition="center" tag="span" />
</Button>
</>
Copy link
Member

Choose a reason for hiding this comment

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

I would simply keep the title without introducing a tooltip

Suggested change
<>
<Tooltip anchor={title}>{title}</Tooltip>
<Button
id={title}
className={classes(actionButtonStyle, className)}
disabled={disabled}
onClick={onClick}
>
<icon.react elementPosition="center" tag="span" />
</Button>
</>
<Button
title={title}
className={classes(actionButtonStyle, className)}
disabled={disabled}
onClick={onClick}
>
<icon.react elementPosition="center" tag="span" />
</Button>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants