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

Don't Wrap Values in Records #3145

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Don't Wrap Values in Records #3145

merged 4 commits into from
Sep 3, 2024

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Sep 3, 2024

Steve pointed out the confusion that comes when the app wraps a non-record in a record to display it in a table format. Now we just display in the inspector view.

The meaning of the "table view" button is now "table view preferred when possible".

Before

CleanShot 2024-09-03 at 12 22 51@2x

After

CleanShot 2024-09-03 at 12 22 38@2x

@jameskerr jameskerr changed the title table nowrap Dponm' Sep 3, 2024
@jameskerr jameskerr changed the title Dponm' Don't Wrap Values in Records Sep 3, 2024
@jameskerr jameskerr requested review from mccanne and philrz September 3, 2024 19:23
@philrz

This comment was marked as resolved.

@philrz
Copy link
Contributor

philrz commented Sep 3, 2024

Here's a bonus thought about what I showed in the last comment, and maybe @mccanne would have some input here.

The app's guidance "...or fuse results to view as a table" is arguably incorrect since with a sequence of primitive values like this fuse can't turn it into a table, and by applying fuse each value will instead just take on a union type. The way the branch is working now, clicking that fuse link just gives the unlabeled sequence of values.

image

If the app had picked up on the presence of any primitive values in the query output, perhaps it could have changed the guidance to more accurately reflect that it's impossible to make into a table and say as much, and perhaps drop the fuse link in that case?

@jameskerr jameskerr merged commit e8a538e into main Sep 3, 2024
4 checks passed
@jameskerr jameskerr deleted the table-nowrap branch September 3, 2024 21:51
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.

2 participants