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.

Sign up for GitHub

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

Jump to bottom

Improvements to the Table visualization. #6653

Merged
merged 8 commits into from May 12, 2023
Merged

Improvements to the Table visualization. #6653

merged 8 commits into from May 12, 2023

Conversation

jdunkerley
Copy link
Member

@jdunkerley jdunkerley commented May 11, 2023 •
edited

Pull Request Description

  • Moved the row count out of the grid.
    • Shown for all cases.
  • Added some of the JS wiring to do pages but currently hidden.
  • Dropdown allowing the user to control the number of rows rendered.
  • Nothing rendered as a italic light Nothing not just empty now.
  • Function rendered as [Function].
  • Rounded corners on top to make it align more with look of panel.
  • Dropdown for Text.split delimiter feed.

Screenshots

image

image

image

image

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@jdunkerley jdunkerley added the CI: No changelog needed Do not require a changelog entry for this PR. label May 11, 2023
@jdunkerley jdunkerley marked this pull request as ready for review May 11, 2023 17:06
radeusgd
radeusgd reviewed May 12, 2023
View reviewed changes
app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js
} else if (params.value === undefined) {
return ''
} else if (params.value === '') {
return '<span style="color:grey; font-style: italic;">Empty</span>'
Copy link
Member

Choose a reason for hiding this comment

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

what do you think about

Suggested change
return '<span style="color:grey; font-style: italic;">Empty</span>'
return '<span style="color:grey; font-style: italic;">Empty Text</span>'

or

Suggested change
return '<span style="color:grey; font-style: italic;">Empty</span>'
return '<span style="color:grey; font-style: italic;">""</span>'

Empty is not that clear to me that this is a text.

Personally I like "" most.

But if you think that for data analysts Empty will be clearer, I'm not against it.

radeusgd
radeusgd approved these changes May 12, 2023
View reviewed changes
Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

Looks good, I'm especially happy about the pretty Nothing visualization in Table.

A few suggestions in line, please take what you like and drop the rest :)

@jdunkerley jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label May 12, 2023
@mergify mergify bot merged commit 00110b8 into develop May 12, 2023
24 of 26 checks passed
@mergify mergify bot deleted the wip/jd/table-viz-tweaks branch May 12, 2023 16:51
Procrat added a commit that referenced this pull request May 15, 2023
@Procrat
Merge branch 'develop' into wip/procrat/shift-space-for-fullscreen-vi…
cc7daae 
…z-6260

* develop:
  Build nightlies every day. (#6681)
  Force `newDashboard` default on the CI-built packages. (#6680)
  Verify ascribed types of parameters really exist (#6584)
  SuggestionBuilder needs to send ascribedType of constructor parameters (#6655)
  Improvements to the Table visualization. (#6653)
  Removing flush (#6670)
  Improving widgets for take/drop (#6641)
@unfurl-links unfurl-links bot mentioned this pull request May 16, 2023
Closed
Procrat added a commit that referenced this pull request May 16, 2023
@Procrat
Merge branch 'develop' into wip/procrat/home-button-6399
030cbe7 
* develop:
  Build nightlies every day. (#6681)
  Force `newDashboard` default on the CI-built packages. (#6680)
  Verify ascribed types of parameters really exist (#6584)
  SuggestionBuilder needs to send ascribedType of constructor parameters (#6655)
  Improvements to the Table visualization. (#6653)
  Removing flush (#6670)
  Improving widgets for take/drop (#6641)
  disable authentication and newDashboard by default (#6664)
  Add COOP+COEP+CORP headers (#6646)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radeusgd radeusgd radeusgd approved these changes

@farmaazon farmaazon farmaazon approved these changes

@GregoryTravis GregoryTravis GregoryTravis approved these changes

@MichaelMauderer MichaelMauderer Awaiting requested review from MichaelMauderer

@wdanilo wdanilo Awaiting requested review from wdanilo

@kazcw kazcw Awaiting requested review from kazcw kazcw is a code owner

Assignees
No one assigned
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jdunkerley @radeusgd @farmaazon @GregoryTravis