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

AQL Table Creation UI #108

Merged
merged 10 commits into from
Aug 12, 2020
Merged

AQL Table Creation UI #108

merged 10 commits into from
Aug 12, 2020

Conversation

jjnesbitt
Copy link
Member

@jjnesbitt jjnesbitt commented Jul 3, 2020

Fixes #106.

This adds the UI for several key features:

  • Running AQL queries
  • Viewing the result of AQL queries
  • Creating tables from the result of AQL queries

A new dependency is added, vue-json-pretty, which allows for viewing the JSON results in a clean way.

@jjnesbitt
Copy link
Member Author

Due to the previews not building, I've included some screenshots:
Screenshot from 2020-07-03 19-29-38
Screenshot from 2020-07-03 19-30-23
Screenshot from 2020-07-03 19-30-53

@jjnesbitt jjnesbitt requested review from waxlamp and jtomeck July 7, 2020 15:14
Copy link
Contributor

@jtomeck jtomeck left a comment

Choose a reason for hiding this comment

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

@AlmightyYakob sorry that I'm just getting to this review. I love it so far, but have a couple thoughts. Let me know if you need clarification on anything I flesh out below

1.) Would it make sense to make the textarea where the AQL is written to take up the full screen (except the sidebars and headerbar of course), and maybe have controls for dark theme text field. I think what you have there is a monospaced/code font, right (I was gonna suggest it if not)?
2.) I'm not sure the right sidebar needs to be done with expansion panels. What if it were 3 evenly sized cards, with separate headings, and lists inside that have scroll bars if the content is too long.
3.) The 3 buttons at the bottom. Can we make the cancel button have the text prop? We also don't really use green buttons anywhere, so can we make the green button grey darken-2 or 3?

I'll let you know if I come up with anything else.

@jjnesbitt
Copy link
Member Author

1.) Would it make sense to make the textarea where the AQL is written to take up the full screen (except the sidebars and headerbar of course), and maybe have controls for dark theme text field. I think what you have there is a monospaced/code font, right (I was gonna suggest it if not)?

  • I'm not sure what you mean by taking up full screen, while still retaining the sidebars and headers. I personally think there's enough space there for the editor.
  • Regarding the dark theme comment, that's not a bad idea, but I think that'd be a separate issue, as that's something we'll want to enable site-wide.
  • I didn't specify a monospaced/code font, but that's a good idea. I'll try that out.

2.) I'm not sure the right sidebar needs to be done with expansion panels. What if it were 3 evenly sized cards, with separate headings, and lists inside that have scroll bars if the content is too long.

Good idea.

3.) The 3 buttons at the bottom. Can we make the cancel button have the text prop? We also don't really use green buttons anywhere, so can we make the green button grey darken-2 or 3?

By cancel, you mean the red clear button? If you feel that keeping the colors consistent with site-wide theme is important enough, then I can update these buttons. However, I do think that these buttons nicely represent the actions with color (red is cancel/clear, blue is a non-modifying action, green is a "submit" action). Just think that's worth considering.

@jjnesbitt jjnesbitt force-pushed the aql-table-creation branch from 48cd947 to 19ad3d2 Compare August 3, 2020 22:27
@jtomeck
Copy link
Contributor

jtomeck commented Aug 4, 2020

I'm not sure what you mean by taking up full screen

I thought taking up the entire area between the two sidebars with a dark theme text box, similar to what you'd see in your code editor would be neat. Not necessary. I think a happy medium would be using a dark theme text box and removing the elevation.

By cancel, you mean the red clear button? If you feel that keeping the colors consistent with site-wide theme is important enough, then I can update these buttons. However, I do think that these buttons nicely represent the actions with color (red is cancel/clear, blue is a non-modifying action, green is a "submit" action). Just think that's worth considering.

Yeah that button. My concern wasn't really about matching the buttons to the theme. There are two schools of thought on secondary action buttons like "cancel" or "clear" or "delete." There are those that feel that they need to have more attention called to them and be colored appropriately in order to let users know "do not click unless you really mean to." (I typically like this school of thought for the "delete" action, but that's pretty much the only one) The school of thought I usually follow is to remove the emphasis from them. The user will typically then ignore that action unless they really need it. Additionally I find it a bit more aesthetically pleasing. Not trying to sound pretentious, but I always call them skittles buttons when there's multicolored action buttons.

That said, that's just my opinion, and neither school of thought is considered right or wrong. So let me know which route you'd like to take! Nice work so far though!

@waxlamp
Copy link
Contributor

waxlamp commented Aug 4, 2020

By cancel, you mean the red clear button? If you feel that keeping the colors consistent with site-wide theme is important enough, then I can update these buttons. However, I do think that these buttons nicely represent the actions with color (red is cancel/clear, blue is a non-modifying action, green is a "submit" action). Just think that's worth considering.

Yeah that button. My concern wasn't really about matching the buttons to the theme. There are two schools of thought on secondary action buttons like "cancel" or "clear" or "delete." There are those that feel that they need to have more attention called to them and be colored appropriately in order to let users know "do not click unless you really mean to." (I typically like this school of thought for the "delete" action, but that's pretty much the only one) The school of thought I usually follow is to remove the emphasis from them. The user will typically then ignore that action unless they really need it. Additionally I find it a bit more aesthetically pleasing. Not trying to sound pretentious, but I always call them skittles buttons when there's multicolored action buttons.

That said, that's just my opinion, and neither school of thought is considered right or wrong. So let me know which route you'd like to take! Nice work so far though!

My vote is for not having skittles buttons 🌈 Let's try out @jtomeck's color sense on this to see what it looks like.

jjnesbitt and others added 6 commits August 7, 2020 10:34
The `modules: true` rule allows for importing of typescript-built
modules such as multinetjs (which includes an `import` statement that
confuses the linter somehow).

The `this: any` workaround avoids yet another instance of the difficulty
of typechecking object-style components in Vue 2.x.
@jjnesbitt jjnesbitt force-pushed the aql-table-creation branch from 43d1bd4 to 42990c2 Compare August 7, 2020 14:38
@jjnesbitt
Copy link
Member Author

jjnesbitt commented Aug 7, 2020

@jtomeck How's this look?

Screenshot from 2020-08-07 12-13-58

@jjnesbitt jjnesbitt marked this pull request as ready for review August 7, 2020 17:09
jtomeck
jtomeck previously approved these changes Aug 10, 2020
Copy link
Contributor

@jtomeck jtomeck left a comment

Choose a reason for hiding this comment

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

Looks awesome, thanks @AlmightyYakob

Copy link
Contributor

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

Thanks for all the work that went into this, @AlmightyYakob!

src/views/AQLWizard.vue Show resolved Hide resolved
@jjnesbitt jjnesbitt merged commit 69122c7 into master Aug 12, 2020
@jjnesbitt jjnesbitt deleted the aql-table-creation branch August 12, 2020 16:22
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.

Add UI for AQL-based table creation
3 participants