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/16 15:13:46 UTC

[GitHub] [airflow] ryanahamilton opened a new pull request #11583: UX Enhancement: Add button to clear search query from DAG search

ryanahamilton opened a new pull request #11583:
URL: https://github.com/apache/airflow/pull/11583


   Currently there is no easy way to reset the DAGs view (home) after performing a search—leaving users stranded. This update adds a button (X) that only displays when a search query is present. This button removes the search param from the page and refreshes the page (while retaining other url params).
   
   ![Screen Recording 2020-10-16 at 11 09 53 AM](https://user-images.githubusercontent.com/3267/96275799-2e3f0c80-0fa0-11eb-9feb-a98935d12135.gif)
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506714600



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -313,6 +316,12 @@ <h2>DAGs</h2>
       }
     });
 
+    $('#search_form').on('reset', function() {
+      var query = new URLSearchParams(window.location.search);
+      query.delete('search');

Review comment:
       I take that back… I was confusing the page param with how the search/tags/status params were being handled. Updated to delete the page param completely.




----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506536914



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -60,9 +60,12 @@ <h2>DAGs</h2>
         </div>
         <div class="col-sm-4">
           <form id="search_form" class="form-inline">
-            <div class="form-group" style="width: 100%;">
+            <div class="form-group search-input" style="width: 100%;">
               <label for="dag_query" class="sr-only">Search DAGs</label>
-              <input type="search" id="dag_query" class="typeahead form-control" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              <input type="search" id="dag_query" class="typeahead form-control search-input__input" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              {% if search_query %}
+                <button type="reset" class="btn btn-default btn-sm material-icons search-input__clear-btn">cancel</button>

Review comment:
       "cancel" is the required string for material icons to render the desired icon. I went ahead and added an `aria-label` attribute instead.




----------------------------------------------------------------
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 a change in pull request #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506567699



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -313,6 +316,12 @@ <h2>DAGs</h2>
       }
     });
 
+    $('#search_form').on('reset', function() {
+      var query = new URLSearchParams(window.location.search);
+      query.delete('search');

Review comment:
       Oh yes, good catch.




----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506536914



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -60,9 +60,12 @@ <h2>DAGs</h2>
         </div>
         <div class="col-sm-4">
           <form id="search_form" class="form-inline">
-            <div class="form-group" style="width: 100%;">
+            <div class="form-group search-input" style="width: 100%;">
               <label for="dag_query" class="sr-only">Search DAGs</label>
-              <input type="search" id="dag_query" class="typeahead form-control" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              <input type="search" id="dag_query" class="typeahead form-control search-input__input" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              {% if search_query %}
+                <button type="reset" class="btn btn-default btn-sm material-icons search-input__clear-btn">cancel</button>

Review comment:
       "cancel" is the required sting for material icons to render the desired icon. I went ahead and added an `aria-label` attribute instead.




----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#issuecomment-710591573


   I've got a follow-up to this PR in the works that will apply this same pattern to the tag filter as well:
   <img width="589" alt="Image 2020-10-16 at 4 38 20 PM" src="https://user-images.githubusercontent.com/3267/96306651-14b4b980-0fce-11eb-908a-ffd6d7d075e7.png">
   


----------------------------------------------------------------
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] kaxil merged pull request #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11583:
URL: https://github.com/apache/airflow/pull/11583


   


----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506572598



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -313,6 +316,12 @@ <h2>DAGs</h2>
       }
     });
 
+    $('#search_form').on('reset', function() {
+      var query = new URLSearchParams(window.location.search);
+      query.delete('search');

Review comment:
       Instead of deleting the `page` param, I had to set it to `0`. Because the page value is maintained in the session cookie, that value was re-added to to the URL after deletion. By setting to `0`, that will update the cookie value as well.




----------------------------------------------------------------
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 a change in pull request #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506566083



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -313,6 +316,12 @@ <h2>DAGs</h2>
       }
     });
 
+    $('#search_form').on('reset', function() {
+      var query = new URLSearchParams(window.location.search);
+      query.delete('search');

Review comment:
       Should we remove page no also? I think the user should always be redirected to the first page.




----------------------------------------------------------------
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 #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506566941



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -313,6 +316,12 @@ <h2>DAGs</h2>
       }
     });
 
+    $('#search_form').on('reset', function() {
+      var query = new URLSearchParams(window.location.search);
+      query.delete('search');

Review comment:
       Ah, yes, good call!




----------------------------------------------------------------
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 a change in pull request #11583: UX Enhancement: Add button to clear search query from DAG search

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11583:
URL: https://github.com/apache/airflow/pull/11583#discussion_r506534125



##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -60,9 +60,12 @@ <h2>DAGs</h2>
         </div>
         <div class="col-sm-4">
           <form id="search_form" class="form-inline">
-            <div class="form-group" style="width: 100%;">
+            <div class="form-group search-input" style="width: 100%;">
               <label for="dag_query" class="sr-only">Search DAGs</label>
-              <input type="search" id="dag_query" class="typeahead form-control" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              <input type="search" id="dag_query" class="typeahead form-control search-input__input" data-provide="typeahead" style="width:100%;" value="{{search_query}}" autocomplete="off" placeholder="Search DAGs">
+              {% if search_query %}
+                <button type="reset" class="btn btn-default btn-sm material-icons search-input__clear-btn">cancel</button>

Review comment:
       ```suggestion
                   <button type="reset" class="btn btn-default btn-sm material-icons search-input__clear-btn">Clear DAG Search Term</button>
   ```
   
   Might make it better if someone tries using a TTS browser?




----------------------------------------------------------------
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