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

Data Explorer: Visually distinguish column names with leading/trailing whitespace or empty string #5653

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

wesm
Copy link
Contributor

@wesm wesm commented Dec 6, 2024

Aims to address #3084, #3089. Related to #5652, we also cannot distinguish empty strings or strings with leading whitespace in column names both in the data grid headers and the summary panel.

This PR applies the same approach we used for data cells to format column names in the grid headers and the summary pane. The appearance is could potentially be a bit odd because we use non-fixed-width fonts.

image

e2e: @:data-explorer

@wesm wesm force-pushed the feature/de-column-names-leading-whitespace branch from 15342be to 8b788ba Compare December 16, 2024 22:24
Copy link

github-actions bot commented Dec 16, 2024

E2E Tests 🚀  ?
This PR will run tests tagged with: @critical @data-explorer

@wesm wesm force-pushed the feature/de-column-names-leading-whitespace branch 2 times, most recently from 8687996 to a15743a Compare December 18, 2024 19:58
@wesm wesm changed the title Data Explorer: Strawman proposal for visually distinguishing column names with leading whitespace or empty string Data Explorer: Visually distinguish column names with leading/trailing whitespace or empty string Dec 19, 2024
@wesm wesm force-pushed the feature/de-column-names-leading-whitespace branch from a15743a to dc14c13 Compare December 19, 2024 16:43
Copy link
Contributor

@juliasilge juliasilge left a comment

Choose a reason for hiding this comment

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

This is a big improvement, and so much more clear! 🙌

  • The leading/trailing whitespace treatment looks great and consistent for both R and Python.
  • The empty string column names for Python look great.
  • The empty string column names for R are still rendering like this:

Screenshot 2024-12-19 at 3 26 26 PM

Does there need to be a backend change for the R case? As far as I can tell, the column name is literally just an empty string so I'm not sure why it may be rendering different.

@wesm wesm force-pushed the feature/de-column-names-leading-whitespace branch from fbb0cb6 to c83ab2f Compare January 3, 2025 00:37
@wesm
Copy link
Contributor Author

wesm commented Jan 3, 2025

Does there need to be a backend change for the R case? As far as I can tell, the column name is literally just an empty string so I'm not sure why it may be rendering different.

Indeed -- Ark is explicitly formatting empty column names like that, so we can change it to return a length-0 string: /~https://github.com/posit-dev/ark/blob/a569d6ee15d85723aec5625620c0593a7a8476a7/crates/ark/src/data_explorer/r_data_explorer.rs#L583

@wesm
Copy link
Contributor Author

wesm commented Jan 3, 2025

I'll go ahead and merge this and then make a PR to adjust the empty column representation returned by Ark

@wesm wesm merged commit 0841d7b into main Jan 3, 2025
6 checks passed
@wesm wesm deleted the feature/de-column-names-leading-whitespace branch January 3, 2025 17:09
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants