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

Minor improvements to SQL Lab UI #5662

Merged
merged 12 commits into from
Aug 20, 2018
Merged

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Aug 17, 2018

Several improvements to the SQL Lab UI

  • Adjust the vertical position of caret in the tab and reduce padding on the right
  • Improve vertical alignment of x and arrow in Select control
  • Separate table dropdown from database and schema because it is not mandatory and also behave differently. Add more description of what it actually does.
  • Move the collapse button in the front. Wrap table names in backticks (``)
  • Align the x button and data types

sqllab-left-editor

  • Align the icons and text in the dropdown menu

sqllab

  • Remove word "for" and use ":" instead to shorten. Also wrap table name in backtick, similar to the schema on the left side.

superset_and_superset

  • Add new table on top instead of bottom. To avoid user not seeing new table added when the current table(s) exceed the screen.

superset_and_superset-2

@williaster @graceguo-supercat @conglei

@codecov-io
Copy link

codecov-io commented Aug 17, 2018

Codecov Report

Merging #5662 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5662      +/-   ##
=========================================
+ Coverage   63.49%   63.5%   +<.01%     
=========================================
  Files         360     360              
  Lines       22889   22891       +2     
  Branches     2549    2551       +2     
=========================================
+ Hits        14534   14536       +2     
  Misses       8340    8340              
  Partials       15      15
Impacted Files Coverage Δ
...t/assets/src/SqlLab/components/CopyQueryTabUrl.jsx 78.57% <ø> (ø) ⬆️
...uperset/assets/src/SqlLab/components/SouthPane.jsx 88.57% <ø> (ø) ⬆️
...ets/src/SqlLab/components/TemplateParamsEditor.jsx 91.83% <ø> (ø) ⬆️
...rset/assets/src/SqlLab/components/TableElement.jsx 95.23% <ø> (ø) ⬆️
superset/assets/src/reduxUtils.js 75.4% <100%> (+0.83%) ⬆️
.../assets/src/SqlLab/components/SqlEditorLeftBar.jsx 92.52% <100%> (ø) ⬆️
.../assets/src/SqlLab/components/TabbedSqlEditors.jsx 90.99% <100%> (ø) ⬆️
superset/assets/src/SqlLab/reducers.js 58.18% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d15d91...e256fd5. Read the comment docs.

@williaster
Copy link
Contributor

These look great! 🙌

I'll 👀 more closely, my only high-level thought for now is about the "See table Schema" change. I thiiiink if you don't add a table, you can't actually query it in the SQL lab editor which is why it was labeled "add table". If that's true I wonder if "add table to workspace" or something would be a better label? Just a thought.

@kristw
Copy link
Contributor Author

kristw commented Aug 18, 2018

I can query without adding a table. Only database and schema are required.

superset

@williaster
Copy link
Contributor

ah got it 👍 (sorry I should have just checked that before flagging)

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

LGTM 💯 ✨

@kristw
Copy link
Contributor Author

kristw commented Aug 18, 2018

Thank you. Can merge when you see fit.

@williaster williaster merged commit cdd348a into apache:master Aug 20, 2018
kristw added a commit to kristw/incubator-superset that referenced this pull request Aug 20, 2018
* Remove "for"

* add space

* Separate control to select table from database and schema.

* Adjust schema displays

* Fix caret and arrow position in Select and Tab

* Reduce space after caret in tab header

* Use translator

* Align icons in the pop-up menu in Sql Lab

* Add new table in front of the list (so it will appear on top)

* shorten message

* reduce line

(cherry picked from commit cdd348a)
@kristw kristw deleted the kristw-sql-lab branch August 22, 2018 18:14
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Remove "for"

* add space

* Separate control to select table from database and schema.

* Adjust schema displays

* Fix caret and arrow position in Select and Tab

* Reduce space after caret in tab header

* Use translator

* Align icons in the pop-up menu in Sql Lab

* Add new table in front of the list (so it will appear on top)

* shorten message

* reduce line
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants