You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/19 14:40:27 UTC
[GitHub] [airflow] ryanahamilton opened a new pull request #11661: UX improvements of DAG tag filtering
ryanahamilton opened a new pull request #11661:
URL: https://github.com/apache/airflow/pull/11661
- Instead of requiring the user to select/deselect tags and then submit the form with the "Filter tags" button, the page now updates as the tag is selected/deselected.
- Removes the storage of the tags in the `tags_filter` cookie. For a filtering pattern such as this, the URL params should always reflect the current result set. Currently, you can have your result set filtered by tags and have no `tags=` params in your URL. This also required using a `&reset_tags=Reset` param to clear the values—this will no longer be needed with the URL explicitly reflecting the values.
- Replaces the "Reset" button with an "X" icon button (same pattern used for search filter, added in #11583). This button only appears when tag filters are applied.
- The sum of these updates yields a much simpler UI where we don't need persistent buttons taking up space.
- Updates Docs screenshot
| Before | After |
|---|---|
|  |  |
CC: @zacharya19 thought you might be interested in these modifications to your feature originally added in #6489.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712303637
@mik-laj I like that idea. It might make sense to save the status (all/active/paused) in the "default" preference as well since we're also cookie-ing that value? That feels like it should be addressed as a feature in a subsequent PR—I think I can offer a refactor to this PR that maintains the current user expectations but still delivers on the UX improvements.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on a change in pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#discussion_r507995461
##########
File path: airflow/www/views.py
##########
@@ -418,12 +418,15 @@ def get_int_arg(value, default=0):
if request.args.get('reset_tags') is not None:
flask_session[FILTER_TAGS_COOKIE] = None
arg_tags_filter = None
+ # Remove the reset_tags=reset from the URL
+ return redirect(url_for('Airflow.index'))
else:
Review comment:
Ah, yes, thanks for edits!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb merged pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #11661:
URL: https://github.com/apache/airflow/pull/11661
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712223585
From a user POV the existing behaviour is very useful though -- espeically if you have a large Airflow deployment, and you only want to see "your team's" DAGs -- having to re-apply the filter each time you visit the page would be tedious.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712222110
@ashb yeah, I understand that desire, but I believe it to be anti-pattern for filtering to not have continuity with the URL params. When a user utilizes the back button, their previously applied tag filters will still persist.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712363316
I reverted the removal of the cookie storage, but I've updated it so that if cookie values exist, the `tags=` param(s) is added to the URL so it always matches the filtered result set.
I also made an update to strip the `reset_tags=reset` from the URL after the cookie value has been cleared.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712216316
I think the reason for the `tags_filter` cookie is so that if you navigate away (say go and view a DAG) and then come back to the home page we want the same filters to apply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zacharya19 commented on a change in pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
zacharya19 commented on a change in pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#discussion_r507981342
##########
File path: airflow/www/views.py
##########
@@ -418,12 +418,15 @@ def get_int_arg(value, default=0):
if request.args.get('reset_tags') is not None:
flask_session[FILTER_TAGS_COOKIE] = None
arg_tags_filter = None
Review comment:
No longer needed.
##########
File path: airflow/www/views.py
##########
@@ -418,12 +418,15 @@ def get_int_arg(value, default=0):
if request.args.get('reset_tags') is not None:
flask_session[FILTER_TAGS_COOKIE] = None
arg_tags_filter = None
+ # Remove the reset_tags=reset from the URL
+ return redirect(url_for('Airflow.index'))
else:
Review comment:
Since you're returning in the if block, you can remove the else condition here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712305469
@ryanahamilton I have no preference. Each solution works for me. Let me know when your change is ready for review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712238423
@ash I see that even though "tags" are typically an unspecified feature use, this feature was originally intended for this "team" use-case, which makes more sense to persist the filter. I'll see about refactoring this to keep the cookie in place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #11661: UX improvements of DAG tag filtering
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11661:
URL: https://github.com/apache/airflow/pull/11661#issuecomment-712297195
I can see the benefits of @ryanahamilton 's solution, but also understand the needs of some users presented by @ashb who wanted selected tags to be remembered at all times. Maybe we can combine these two ideas and let the user keep the currently selected tags as default? I see it this way, that the user can browse DAGs normally according to @ryanahamilton 's suggestion, but if user wants to configure, be able to quickly return to the selected configuration, all users has to do is press the "Save as default" button. It doesn't even have to be on this page, but it could be a configuration option in your user profile settings ("Default selected tags" or something similar). I think this will better address the needs of both user groups.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org