Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

SQLTable: add row filters #393

Merged
merged 5 commits into from
Mar 3, 2018
Merged

Conversation

n-riesco
Copy link
Contributor

@n-riesco n-riesco commented Mar 1, 2018

  • Replaced fixed-data-table with react-data-grid

  • Implemented row filters

Closes #351


@tarzzz Lately, I've been asking you to review only PRs for the backend. Would you like to review this PR?

@jackparmer Would you take this PR for a spin and see if you'd like to change anything?

* Replaced `fixed-data-table` with `react-data-grid`.

* Enable/Disable row filters by clicking on the header.
* Removed dependency `fixed-data-table`.
@jackparmer
Copy link
Contributor

@n-riesco I'm having trouble running the artifact. Could you share a screencast?

@n-riesco
Copy link
Contributor Author

n-riesco commented Mar 2, 2018

@jackparmer could you describe what the trouble was? was it hard to discover? should I move the message (click on the header to toggle the row filter) to the top? were you using a phone?

peek 2018-03-02 08-17

@jackparmer
Copy link
Contributor

Thanks! I think this looks good.

@@ -5,6 +5,8 @@
<meta charset="utf-8">
<title>Falcon SQL Client</title>

<link rel="stylesheet" type="text/css" href="static/css/falcon.css"></link>
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably too late to change; however, is there a reason we have separate css files and not using something webpack to bundle this. Again not important just an observation

Copy link
Contributor

Choose a reason for hiding this comment

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

@n-riesco Looks good as well 💃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shannonlal

Probably too late to change; however, is there a reason we have separate css files and not using something webpack to bundle this. Again not important just an observation

This is a temporary solution. I'd like to come back to packing CSS with webpack and possibly using http://lesscss.org/ to define a some sort of theme to be used by all components.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to come back to packing CSS with webpack and possibly using http://lesscss.org/ to define a some sort of theme to be used by all components.

I've heard good things about Tachyons, which seems to be what SQLPad is using. Should be easy enough to harmonize something like this with plotly's styleguide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jack, as usual, your collections of links are gold! :)

@n-riesco n-riesco merged commit 11d2712 into plotly:master Mar 3, 2018
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.

Allow column filtering in table
3 participants