pairwise-tech / pairwise

The Pairwise Codebase
https://app.pairwise.tech
6 stars 0 forks source link

Show SQL Results as Table #272

Closed no-stack-dub-sack closed 3 years ago

no-stack-dub-sack commented 3 years ago

I thought it would be an improvement to show SQL results as a table, more similar to what a student would encounter when working in a RDBMS like SSMS, DBeaver, etc.

Here's a basic idea of what this would look like:

image

Note that this relies on the console data passed back from the iframe. Currently, this will only work if the results from the SQL query are logged from the test code. There's some not great code to isolate logs that represent SQL results from other logs so that user logs will not interfere. Still, I wonder if there's a better and somewhat easy way to get these results other than using the log infra...

no-stack-dub-sack commented 3 years ago

@bonham000 Let me know what you think of where this stands. I think it's at a decent place for the time being. I added support for showing results multiple tables (pictured at the bottom), and made sure everything works / looks good in mobile view.

I didn't want to spend too much time on perfecting this since I do want to move on to the content itself (and am not getting too much time each week to dive in), and I figure we can improve this as we go (especially since this is pretty low-traffic content for now, so the need for perfection isn't high). But there are some nice-to-haves that are not yet implemented and questions I'd eventually like to answer:

Also, when we made the change to show the test cases / preview test results, the EmptyPreviewCoverPanel never rendered because there was never an empty test case array. I updated the logic to show this once again if we're in test case "preview mode". Do you want to keep this, or leave it as-is for now? It's a nice default for the SQL challenges, but may not serve much of a purpose elsewhere.

image

bonham000 commented 3 years ago

@no-stack-dub-sack

I think the EmptyPreviewCoverPanel changes are fine 👍

Feel free to merge this if you are happy with it.

no-stack-dub-sack commented 3 years ago

@bonham000 I'll explain what I mean in Slack, which I think will be easier.

And sounds good, I'll give it one more quick review and then merge in the morning once I fix whatever's causing CI to fail.

no-stack-dub-sack commented 3 years ago

If we ever come back to the custom styling, had a bit of luck overriding some of the styles with the following CSS, but there's so much going on in the table that more time is needed with this:

.bp3-table-menu,
.bp3-table-header,
.bp3-table-row-name,
.bp3-table-quadrant,
.bp3-table-container,
.bp3-table-column-headers {
  background-color: rgb(36, 36, 36) !important;
}

.bp3-table-resize-handle,
.bp3-table-resize-handle-target,
.bp3-dark .bp3-table-cell-client {
  background-color: rgb(30, 30, 30) !important;
}