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

feat: Update nav bar with new data information architecture for SQL Lab + SQL #21245

Merged
merged 15 commits into from
Sep 8, 2022
Merged

feat: Update nav bar with new data information architecture for SQL Lab + SQL #21245

merged 15 commits into from
Sep 8, 2022

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Aug 29, 2022 •
edited
Loading

SUMMARY

Updating the data information architecture for the nav bar to match the new designs from product. Mostly renaming SQL LAB -> SQL + SQL Editor to SQL LAB. This PR doesn't change the underlying permissions but does, update the labels and layout there will be a follow up PR once for renaming once we are getting closer to 3.0 cut

Screen Shot 2022-09-02 at 11 16 03 AM

Screen Shot 2022-09-02 at 11 16 11 AM

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@hughhhh hughhhh requested a review from a team as a code owner August 29, 2022 22:23
@codecov
Copy link

codecov bot commented Aug 29, 2022 •
edited
Loading

Codecov Report

Merging #21245 (cb9741b) into master (99a4f05) will decrease coverage by 11.26%.
The diff coverage is 63.88%.

❗ Current head cb9741b differs from pull request most recent head 1651246. Consider uploading reports for the commit 1651246 to get more accurate results

@@             Coverage Diff             @@
##           master   #21245       +/-   ##
===========================================
- Coverage   66.45%   55.18%   -11.27%     
===========================================
  Files        1789     1790        +1     
  Lines       68296    68415      +119     
  Branches     7275     7287       +12     
===========================================
- Hits        45387    37757     -7630     
- Misses      21034    28782     +7748     
- Partials     1875     1876        +1
Flag Coverage Δ
hive 52.95% <36.95%> (-0.07%) ⬇️
mysql ?
postgres ?
presto 52.85% <36.95%> (-0.07%) ⬇️
python 57.82% <55.79%> (-23.54%) ⬇️
sqlite ?
unit 50.86% <55.79%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...cy-preset-chart-nvd3/src/TimePivot/controlPanel.ts 33.33% <0.00%> (-16.67%) ⬇️
...lLab/components/ExploreCtasResultsButton/index.tsx 8.33% <0.00%> (ø)
...c/SqlLab/components/ExploreResultsButton/index.tsx 100.00% <ø> (ø)
...rontend/src/SqlLab/components/QueryTable/index.tsx 70.17% <ø> (ø)
...frontend/src/SqlLab/components/SouthPane/index.tsx 66.66% <ø> (ø)
...end/src/components/Datasource/DatasourceEditor.jsx 65.35% <0.00%> (-0.26%) ⬇️
...onents/ErrorMessage/ErrorMessageWithStackTrace.tsx 80.00% <ø> (+20.00%) ⬆️
...-frontend/src/components/FilterableTable/index.tsx 72.63% <ø> (ø)
...tend/src/dashboard/components/AnchorLink/index.tsx 93.75% <ø> (+12.50%) ⬆️
...src/dashboard/components/PropertiesModal/index.tsx 61.81% <0.00%> (-0.38%) ⬇️
... and 341 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hughhhh hughhhh closed this Sep 1, 2022
@hughhhh hughhhh reopened this Sep 1, 2022
@yousoph
Copy link
Member

yousoph commented Sep 1, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

@yousoph Ephemeral environment spinning up at http://35.164.66.14:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@betodealmeida
Copy link
Member

Is this changing the UI? Can you add a summary and screenshots as well if it's the case?

@pull-request-size pull-request-size bot added size/M and removed size/L labels Sep 2, 2022
@pull-request-size pull-request-size bot added size/M and removed size/XL labels Sep 2, 2022
@hughhhh
Copy link
Member Author

hughhhh commented Sep 2, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

@hughhhh Ephemeral environment spinning up at http://52.25.211.220:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Antonio-RiveroMartnez
Antonio-RiveroMartnez approved these changes Sep 2, 2022
View reviewed changes
Copy link
Member

@Antonio-RiveroMartnez Antonio-RiveroMartnez left a comment

Choose a reason for hiding this comment

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

LGTM!

eschutho
eschutho reviewed Sep 2, 2022
View reviewed changes
superset/initialization/__init__.py
href="/superset/sqllab/",
category_icon="fa-flask",
icon="fa-flask",
category="SQL Lab",
category_label=__("SQL Lab"),
category_label=__("SQL"),
Copy link
Member

Choose a reason for hiding this comment

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

is this currently using the menu permissions or the can sql lab permissions?

Copy link
Member Author

@hughhhh hughhhh Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

it's using the menu permissions for Sql Lab,

@hughhhh
Copy link
Member Author

hughhhh commented Sep 2, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

@hughhhh Ephemeral environment spinning up at http://54.186.164.86:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

dpgaspar
dpgaspar reviewed Sep 5, 2022
View reviewed changes
Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

can you include your change on #21297 into this PR?

@hughhhh hughhhh merged commit b302818 into master Sep 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2022

Ephemeral environment shutdown and build artifacts deleted.

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
@mistercrunch mistercrunch deleted the data-ia-navbar-2 branch March 26, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eschutho eschutho eschutho left review comments

@dpgaspar dpgaspar dpgaspar left review comments

@AAfghahi AAfghahi AAfghahi approved these changes

@Antonio-RiveroMartnez Antonio-RiveroMartnez Antonio-RiveroMartnez approved these changes

@jinghua-qa jinghua-qa Awaiting requested review from jinghua-qa

@craig-rueda craig-rueda Awaiting requested review from craig-rueda

@villebro villebro Awaiting requested review from villebro

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina

@geido geido Awaiting requested review from geido

@ktmud ktmud Awaiting requested review from ktmud

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@hughhhh @yousoph @betodealmeida @dpgaspar @eschutho @Antonio-RiveroMartnez @AAfghahi @mistercrunch