8000 fix: Missing check indicator when moving Spaces into root level by tatianainama · Pull Request #14868 · lightdash/lightdash · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Missing check indicator when moving Spaces into root level #14868

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tatianainama
Copy link
Contributor

Closes: #14857

Description:

Display check ✓ icon when selecting root + some other style improvements:

  • Unify tree box padding
  • Balance paddings and margins of selected elements in tree (see screenshots comparing hover & selected states)
  • Always destination space, even when moving into Root Space
Before After
image image

Reviewer actions

  • I have manually tested the changes in the preview environment
  • I have reviewed the code
  • I understand that "request changes" will block this PR from merging

@owlas owlas requested a deployment to fix/14857-missing-check-indicator - jaffle_db_pg_13 PR #14868 May 19, 2025 11:27 — with Render Abandoned
@owlas owlas deployed to fix/14857-missing-check-indicator - headless-browser PR #14868 May 19, 2025 11:27 — with Render Active
@joaoviana joaoviana self-requested a review May 19, 2025 11:30
@joaoviana
Copy link
Contributor

Should we make sure that the "summary" message is always visible?
image

Or should this summary message be smaller and somewhere else?

@@ -109,7 +104,7 @@ const TreeItem: React.FC<Props> = ({
{stringLabel}
</Highlight>

{!isRoot && selected && (
{selected && (
Copy link
Contributor

Choose a reason for hiding this comment

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

image

Should we highlight the row as well when 'All spaces' is selected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you shouldn't be able to move a chart into the root space (only spaces) 👀
Working on a fix

@owlas owlas deployed to fix/14857-missing-check-indicator - headless-browser PR #14868 May 22, 2025 17:10 — with Render Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing check indicator when moving Spaces into root level
3 participants
0