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

Add Table List Refresh Button #7502

Conversation

MarcusSorealheis
Copy link
Contributor

@MarcusSorealheis MarcusSorealheis commented May 14, 2019

CATEGORY

Choose one

  • Bug Fix

  • Enhancement (new features, refinement)

SUMMARY

When there are no filters active, to see the filter-less list of tables, you have to do a browser refresh. It would be helpful in the UI for some business users to be able to simply refresh with the search button, but it goes away. Search here is really refresh (usually with filters enabled), so it seems natural.

That is the main change here.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

cannot get fab to update template.

TEST PLAN

Refresh the template through re-initialization of the app and render the new table search page.

ADDITIONAL INFORMATION

  • Has associated issue: #7493

  • Changes UI

REVIEWERS

@codecov-io
Copy link

codecov-io commented May 14, 2019

Codecov Report

Merging #7502 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7502      +/-   ##
==========================================
+ Coverage   65.42%   65.43%   +0.01%     
==========================================
  Files         435      432       -3     
  Lines       21325    21319       -6     
  Branches     2347     2347              
==========================================
  Hits        13951    13951              
+ Misses       7258     7252       -6     
  Partials      116      116
Impacted Files Coverage Δ
...ssets/src/visualizations/presets/MapChartPreset.js
...ts/src/visualizations/presets/CommonChartPreset.js
...ts/src/visualizations/presets/LegacyChartPreset.js
...src/visualizations/presets/HierarchyChartPreset.js
...et/assets/src/visualizations/presets/MainPreset.js 0% <0%> (ø)

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 a4392c8...81c3ec7. Read the comment docs.

@mistercrunch
Copy link
Member

Please share before/after screenshots

@MarcusSorealheis
Copy link
Contributor Author

Please share before/after screenshots

I would like to, however, this is related to the issue where I cannot force the re-rendering of templates of the application in places that are managed by FAB. Let me see if I can check out this branch in a different virtual environment and try to do it.

@MarcusSorealheis
Copy link
Contributor Author

Got it. Screenshots incoming.

@MarcusSorealheis
Copy link
Contributor Author

MarcusSorealheis commented May 14, 2019

Place to start (filters added):
http://localhost:8088/tablemodelview/list/?_flt_0_database=1

Old State:

When you add filters the search button appears:
With fliters

When you clear those filters, as you can see in the screenshot below you are stuck with a filtered list of tables:
with filters cleared

In the current state the only way to render an unfiltered list of list of tables is to refresh the page, which would be step 3 but I am omitting because it would require a refresh and I'm trying to draw the distinction. That's unintuitive to some business users. Since the cursor is already there, it would be better if they could simply refresh with the same button. One could argue that clearing filters prompts a refresh, but if you have as many tables as we do, you don't want to do that unless you want to do that if you catch my drift.

So, here's the new version from this PR:

start with a filter:
Screen Shot 2019-05-14 at 10 54 14 AM

Clear the filter and you still have control (see new button)
Screen Shot 2019-05-14 at 10 54 23 AM

And after clicking the refresh button (you get unfiltered results)

Screen Shot 2019-05-14 at 10 54 31 AM

:

Let me know if this makes sense.

@mistercrunch
Copy link
Member

mistercrunch commented May 14, 2019

Curious why don't you have the superset css theme active here? It'll be important to see this working with the default theme to validate.

@MarcusSorealheis
Copy link
Contributor Author

MarcusSorealheis commented May 15, 2019 via email

@MarcusSorealheis
Copy link
Contributor Author

Ok, @mistercrunch so finally got the original theme working again, where things look a lot better.

Step 1 Filters Applied to Tables Result (intentionally limit the results):
Filters Applied to Tables Results

Step 2 Before Refreshing after filters are cleared (results limited to main tables):
Before Refreshing after filters cleared

Step 3 After clicking refresh w/ filters cleared (results include all tables):
After Clicking refresh button w: filters removed

@MarcusSorealheis
Copy link
Contributor Author

MarcusSorealheis commented May 24, 2019

Ok, @mistercrunch so finally got the original theme working again, where things look a lot better.

Step 2 Before Refreshing after filters are cleared (results limited to main tables):
Before Refreshing after filters cleared

Before this change, the list would not be unfiltered and could not be refreshed in the UI. Users encounter confusion when they clear filters and do not see unfiltered list. On the other hand, and the reason why I added the button, automatically refreshing the table list was a poor experience because some customers have a lot of tables. Refreshing should be deliberate.

The button gives the user a cue that they can refresh if they would like to see unfiltered table list whether with the button or a reload.

@MarcusSorealheis MarcusSorealheis changed the title Keep Search button because previously it disappeared + formatting. Keep Table Filter Refresh Button May 24, 2019
@MarcusSorealheis MarcusSorealheis changed the title Keep Table Filter Refresh Button Add Table List Refresh Button May 24, 2019
@mistercrunch mistercrunch merged commit 56eac68 into apache:master Jul 2, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 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 size/M 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants