Skip to content
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

fix(explore): column data type tooltip format #30588

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Oct 12, 2024

Minor visual tweak to add a column and a space inside the Explore -> LeftPanelDatasetView -> Column datatype tooltip.

Adding a colon and a space, and showing the data type as uppercase as it's pretty standard to expose it in that way elsewhere in Superset and as a general convention

before

Screenshot 2024-10-11 at 5 47 05 PM

after

Screenshot 2024-10-11 at 5 48 28 PM

@@ -55,8 +55,7 @@ const TooltipSection = ({
text: ReactNode;
}) => (
<TooltipSectionWrapper>
<TooltipSectionLabel>{label}</TooltipSectionLabel>
<span>{text}</span>
<TooltipSectionLabel>{label}</TooltipSectionLabel>: <span>{text}</span>
Copy link
Member

Choose a reason for hiding this comment

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

might be a legit use case for a &nbsp;

Copy link
Member

Choose a reason for hiding this comment

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

Also not sure the span has any real purpose here. ¯\_(ツ)_/¯

Copy link
Member Author

Choose a reason for hiding this comment

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

I had a {' '} and I think prettier changed it to what is now there. Generally I prefer the {' '} to &nbsp;, and if prettier is configured to do what it did, let's roll with it. About the span, I think it's referenced in the CSS so that it's not bolded.

@github-actions github-actions bot added i18n Namespace | Anything related to localization risk:db-migration PRs that require a DB migration api Related to the REST API doc Namespace | Anything related to documentation plugins dependencies:npm github_actions Pull requests that update GitHub Actions code labels Oct 28, 2024
@@ -23,7 +23,7 @@ NOTE: This file is generated by helm-docs: https://github.com/norwoodj/helm-docs

# superset

![Version: 0.12.13](https://img.shields.io/badge/Version-0.12.13-informational?style=flat-square)
![Version: 0.13.0](https://img.shields.io/badge/Version-0.13.0-informational?style=flat-square)
Copy link
Member

@rusackas rusackas Oct 29, 2024

Choose a reason for hiding this comment

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

Just curious, why did we have a minor bump? Still figuring out the rhyme or reason of helm version numbers.

n/m it's the addition of extraLablels... I see.

requirements/base.txt Outdated Show resolved Hide resolved
@rusackas
Copy link
Member

And here I am adding review comments all over what looks like a funky rebase. I was about to comment "what is the scope of this PR?" 🤦

@mistercrunch
Copy link
Member Author

Oops, gitfu fail, got thrown off by a local rebate + merged a commit from the PR

Minor visual tweak to add a column and a space inside the
Explore -> LeftPanelDatasetView -> Column datatype tooltip
@mistercrunch mistercrunch force-pushed the explore_left_metadata_type branch from 230c5f5 to 2b54e6d Compare October 29, 2024 16:04
@github-actions github-actions bot removed i18n Namespace | Anything related to localization risk:db-migration PRs that require a DB migration api Related to the REST API doc Namespace | Anything related to documentation plugins dependencies:npm github_actions Pull requests that update GitHub Actions code labels Oct 29, 2024
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

I love how short this is now :)

@mistercrunch mistercrunch merged commit 73768f6 into master Oct 30, 2024
33 of 34 checks passed
@sadpandajoe sadpandajoe added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Nov 13, 2024
sadpandajoe pushed a commit that referenced this pull request Nov 13, 2024
sadpandajoe pushed a commit that referenced this pull request Nov 13, 2024
michael-s-molina pushed a commit that referenced this pull request Nov 19, 2024
@mistercrunch mistercrunch deleted the explore_left_metadata_type branch November 25, 2024 06:39
@mistercrunch mistercrunch added 🍒 4.1.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Nov 27, 2024
betodealmeida pushed a commit that referenced this pull request Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels explore:design Related to the Explore UI/UX packages preset-io size/S v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch 🍒 4.1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants