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/09/14 21:36:24 UTC

[GitHub] [airflow] ryanahamilton opened a new pull request #10944: Task Instance Modal UX Enhancements

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


   Attempting to make one of the more complex UI interactions more intuitive with the following edits:
   
   ### Task Instance Modal
   
   - Adds "X" button to close modal in the top-right as intended by Bootstrap patterns ("Close" button in the bottom-right still persists as well).
   - Cleaned up the presentation the modal header by adding "Task Instance/at" label/prefixes and moving the very randomly placed Filter (icon) button down with the options below as a "Filter Upstream" button. 
   - Removed the "primary" button styling from each of the buttons in the group row ("Task Instance Details", "Rendered", etc.). This wasn't appropriate usage of a "primary" styling as that designation is used to establish a visual hierarchy—in this case all of the actions are considered comparable.
   - Re-ordered each "action" form. (buttons on right instead of left) Forms should flow top to bottom OR left to right. This aligns with the user flow of electing the related options and then submitting the form.
   - Improved the accessibility of the "action" forms by refactoring the underlying markup/scripting to each be wrapped in their own `<form></form>` tags so that they can be individually submitted via keyboard (depending on focus).
   - Updated the screenshot of this modal [in the docs](https://airflow.apache.org/docs/stable/ui.html#task-instance-context-menu).
   
   | Before | After |
   |---|---|
   | ![image](https://user-images.githubusercontent.com/3267/93137393-630f2980-f6ab-11ea-9d22-ac568db826f8.png)  |  ![image](https://user-images.githubusercontent.com/3267/93137153-06137380-f6ab-11ea-871d-1470365982fe.png) |
   
   ### DAG Modal
   Applies patterns consistent with updates to the Task Instance Modal:
   - Added "DAG:" prefix to the modal header
   - Added "X" close button in top-right of modal
   
   | Before | After |
   |---|---|
   | ![image](https://user-images.githubusercontent.com/3267/93137258-2a6f5000-f6ab-11ea-9be4-46b6ca4907bc.png)|  ![image](https://user-images.githubusercontent.com/3267/93137192-14618f80-f6ab-11ea-9c6e-304c4b089b2e.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] turbaszek commented on pull request #10944: Task Instance Modal UX Enhancements

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


   > ![image](https://user-images.githubusercontent.com/3267/93210737-1fa9cf00-f72e-11ea-98f9-34a3194613f4.png)
   
   I like it 👌 
   


----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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


   > Removed the "primary" button styling from each of the buttons in the group row ("Task Instance Details", "Rendered", etc.). This wasn't appropriate usage of a "primary" styling as that designation is used to establish a visual hierarchy—in this case all of the actions are considered comparable.
   
   This is the only change I'm not sure of, as the "primary" styling was used to differentiate toggles from actions, and now it's not visually obvious that the "Task Instance Details" button is an action, but the "Ignore All Deps" is a toggle.


----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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


   


----------------------------------------------------------------
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] himabindu07 commented on pull request #10944: Task Instance Modal UX Enhancements

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


   I have verified with this image quay.io/astronomer/ap-airflow-dev:2.0.0-buster-onbuild-22919


----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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



##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -108,199 +108,221 @@ <h4 class="pull-right">
   </div>
   <hr>
   <!-- Modal -->
-  <div class="modal fade" id="myModal"
-        tabindex="-1" role="dialog"
-      aria-labelledby="myModalLabel" aria-hidden="true">
+  <div class="modal fade" id="taskInstanceModal" tabindex="-1" role="dialog" aria-labelledby="taskInstanceModalLabel" aria-hidden="true">
     <div class="modal-dialog">
       <div class="modal-content">
         <div class="modal-header">
-          <h4 class="modal-title" id="myModalLabel">
-            <span id='task_id'></span>
-            <button id="btn_filter" type="button" class="btn btn-default" title="Filter on this task and upstream ">
-              <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
-            </button>
-              on
-            <span id='execution_date'></span>
+          <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
+          <h4 class="modal-title" id="taskInstanceModalLabel">
+            <span class="text-muted">Task Instance:</span> <span id="task_id"></span>
+            <br /><span class="text-muted">at:</span> <span id="execution_date"></span>
           </h4>
         </div>
         <div class="modal-body">
           <div id="div_btn_subdag">
-            <a id="btn_subdag" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.get_default_view()) }}">
+            <a id="btn_subdag" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.get_default_view()) }}">
               Zoom into Sub DAG
             </a>
             <hr/>
           </div>
-          <a id="btn_task" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.task') }}">
+          <a id="btn_task" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.task') }}">
             Task Instance Details
           </a>
-          <a id="btn_rendered" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.rendered') }}">
+          <a id="btn_rendered" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.rendered') }}">
             Rendered
           </a>
-          <a id="btn_ti" type="button" class="btn btn-primary" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
+          <a id="btn_ti" class="btn btn-sm btn-default" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
             Task Instances
           </a>
-          <a id="btn_log" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.log') }}">
+          <a id="btn_log" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.log') }}">
             View Log
           </a>
+          <button id="btn_filter" type="button" class="btn btn-sm btn-default" title="Filter on this task and upstream ">
+            <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
+            Filter Upstream
+          </button>
           <hr/>
           <div id="dag_dl_logs">
-            <label style="display:inline"> Download Log (by attempts): </label>
+            <label style="display:inline"> Download Log (by attempts):</label>
             <ul class="nav nav-pills" role="tablist" id="try_index" style="display:inline">
             </ul>
             <hr/>
             <hr/>
           </div>
-          <div id="dag_redir_logs">
-            {% if external_log_name is defined %}
-              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts): </label>
+          {% if external_log_name is defined %}
+            <div id="dag_redir_logs">
+              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts):</label>
               <ul class="nav nav-pills" role="tablist" id="redir_log_try_index" style="display:inline">
               </ul>
               <hr/>
               <hr/>
-            {% endif %}
-          </div>
-          <form method="POST">
+            </div>
+          {% endif %}
+          <form method="POST" data-action="{{ url_for('Airflow.run') }}">

Review comment:
       Why `data-action` and not just `action`?




----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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


   @ashb in a view of 27 buttons visual differentiation is certainly going to be a difficult task regardless of the solution. The attached formation of the Radio Button Groups are meant to differentiate their intended use from the detached/spaced presentation of the row at the top. I'm thinking we might be able to more explicitly separate the user concerns by adding a "Task Actions" heading as a demarcation between them. WDYT? 
   
   ![image](https://user-images.githubusercontent.com/3267/93210737-1fa9cf00-f72e-11ea-98f9-34a3194613f4.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] ryanahamilton commented on a change in pull request #10944: Task Instance Modal UX Enhancements

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



##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -108,199 +108,221 @@ <h4 class="pull-right">
   </div>
   <hr>
   <!-- Modal -->
-  <div class="modal fade" id="myModal"
-        tabindex="-1" role="dialog"
-      aria-labelledby="myModalLabel" aria-hidden="true">
+  <div class="modal fade" id="taskInstanceModal" tabindex="-1" role="dialog" aria-labelledby="taskInstanceModalLabel" aria-hidden="true">
     <div class="modal-dialog">
       <div class="modal-content">
         <div class="modal-header">
-          <h4 class="modal-title" id="myModalLabel">
-            <span id='task_id'></span>
-            <button id="btn_filter" type="button" class="btn btn-default" title="Filter on this task and upstream ">
-              <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
-            </button>
-              on
-            <span id='execution_date'></span>
+          <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
+          <h4 class="modal-title" id="taskInstanceModalLabel">
+            <span class="text-muted">Task Instance:</span> <span id="task_id"></span>
+            <br /><span class="text-muted">at:</span> <span id="execution_date"></span>
           </h4>
         </div>
         <div class="modal-body">
           <div id="div_btn_subdag">
-            <a id="btn_subdag" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.default_view) }}">
+            <a id="btn_subdag" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.default_view) }}">
               Zoom into Sub DAG
             </a>
             <hr/>
           </div>
-          <a id="btn_task" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.task') }}">
+          <a id="btn_task" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.task') }}">
             Task Instance Details
           </a>
-          <a id="btn_rendered" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.rendered') }}">
+          <a id="btn_rendered" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.rendered') }}">
             Rendered
           </a>
-          <a id="btn_ti" type="button" class="btn btn-primary" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
+          <a id="btn_ti" class="btn btn-sm btn-default" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
             Task Instances
           </a>
-          <a id="btn_log" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.log') }}">
+          <a id="btn_log" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.log') }}">
             View Log
           </a>
+          <button id="btn_filter" type="button" class="btn btn-sm btn-default" title="Filter on this task and upstream ">
+            <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
+            Filter Upstream
+          </button>
           <hr/>
           <div id="dag_dl_logs">
-            <label style="display:inline"> Download Log (by attempts): </label>
+            <label style="display:inline"> Download Log (by attempts):</label>
             <ul class="nav nav-pills" role="tablist" id="try_index" style="display:inline">
             </ul>
             <hr/>
             <hr/>
           </div>
-          <div id="dag_redir_logs">
-            {% if external_log_name is defined %}
-              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts): </label>
+          {% if external_log_name is defined %}
+            <div id="dag_redir_logs">
+              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts):</label>
               <ul class="nav nav-pills" role="tablist" id="redir_log_try_index" style="display:inline">
               </ul>
               <hr/>
               <hr/>
-            {% endif %}
-          </div>
-          <form method="POST">
+            </div>
+          {% endif %}
+          <form method="POST" data-action="{{ url_for('Airflow.run') }}">
             <input name="csrf_token" type="hidden" value="{{ csrf_token() }}"/>
-            <input name="dag_id" type="hidden">
+            <input name="dag_id" type="hidden" value="{{ dag.dag_id }}">
             <input name="task_id" type="hidden">
             <input name="execution_date" type="hidden">
             <input name="origin" type="hidden" value="{{ request.base_url }}">
-            <button id="btn_run" type="button" class="btn btn-primary"
-              title="Runs a single task instance"
-              data-action="{{ url_for('Airflow.run') }}">
-              Run
-            </button>
-            <span class="btn-group" data-toggle="buttons">
-              <label
-                class="btn"
-                title="Ignores all non-critical dependencies, including task state and task_deps">
-                <input type="checkbox" value="true" name="ignore_all_deps" autocomplete="off">
-                Ignore All Deps</label>
-              <label class="btn"
-                title="Ignore previous success/failure">
-                <input type="checkbox" value="true" name="ignore_ti_state" autocomplete="off">
-                Ignore Task State
-              </label>
-              <label class="btn"
-                title="Disregard the task-specific dependencies, e.g. status of upstream task instances and depends_on_past">
-                <input type="checkbox" value="true" name="ignore_task_deps" autocomplete="off">
-                Ignore Task Deps
-              </label>
-            </span>
-            <hr/>
-            <button id="btn_clear" type="button" class="btn btn-primary"
-                title="Clearing deletes the previous state of the task instance, allowing it to get re-triggered by the scheduler or a backfill command"
-                data-action="{{ url_for('Airflow.clear') }}"
-                >Clear
-            </button>
-            <span class="btn-group" data-toggle="buttons">
-              <label class="btn"
-                title="Also include past task instances when clearing this one">
-                <input type="checkbox" value="true" name="past" autocomplete="off">
-                Past
-              </label>
-              <label class="btn"
-                title="Also include future task instances when clearing this one">
-                <input type="checkbox" value="true" name="future" autocomplete="off">
-                Future
-              </label>
-              <label class="btn"
-                title="Also include upstream dependencies">
-                <input type="checkbox" value="true" name="upstream" autocomplete="off">
-                Upstream
-              </label>
-              <label class="btn active"
-                title="Also include downstream dependencies">
-                <input type="checkbox" value="true" name="downstream" checked autocomplete="off">
-                Downstream
-              </label>
-              <label class="btn active">
-                <input type="checkbox" value="true" name="recursive" checked autocomplete="off">
-                 Recursive
-              </label>
-              <label class="btn"
-                title="Only consider failed task instances when clearing this one">
-                <input type="checkbox" value="true" name="only_failed" autocomplete="off">
-                Failed
-              </label>
-            </span>
-            <hr/>
-            <button id="btn_failed" type="button" class="btn btn-primary"
-                data-action="{{ url_for('Airflow.failed') }}">
-              Mark Failed
-            </button>
-            <span class="btn-group" data-toggle="buttons">
-              <label class="btn">
-                <input type="checkbox" value="true" name="failed_past" autocomplete="off">
-                Past
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="failed_future" autocomplete="off">
-                Future
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="failed_upstream" autocomplete="off">
-                Upstream
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="failed_downstream" autocomplete="off">
-                Downstream
-              </label>
-            </span>
-            <hr/>
-            <button id="btn_success" type="button" class="btn btn-primary"
-                data-action="{{ url_for('Airflow.success') }}">
-              Mark Success
-            </button>
-            <span class="btn-group" data-toggle="buttons">
-              <label class="btn">
-                <input type="checkbox" value="true" name="success_past" autocomplete="off">
-                Past
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="success_future" autocomplete="off">
-                Future
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="success_upstream" autocomplete="off">
-                Upstream
-              </label>
-              <label class="btn">
-                <input type="checkbox" value="true" name="success_downstream" autocomplete="off">
-                Downstream
-              </label>
-            </span>
+            <div class="row">
+              <span class="btn-group col-md-9" data-toggle="buttons">
+                <label
+                  class="btn btn-default"
+                  title="Ignores all non-critical dependencies, including task state and task_deps">
+                  <input type="checkbox" value="true" name="ignore_all_deps" autocomplete="off">
+                  Ignore All Deps</label>
+                <label class="btn btn-default"
+                  title="Ignore previous success/failure">
+                  <input type="checkbox" value="true" name="ignore_ti_state" autocomplete="off">
+                  Ignore Task State
+                </label>
+                <label class="btn btn-default"
+                  title="Disregard the task-specific dependencies, e.g. status of upstream task instances and depends_on_past">
+                  <input type="checkbox" value="true" name="ignore_task_deps" autocomplete="off">
+                  Ignore Task Deps
+                </label>
+              </span>
+              <span class="col-md-3">
+                <button id="btn_run" type="submit" class="btn btn-primary btn-block" title="Runs a single task instance">
+                  Run
+                </button>
+              </span>
+            </div>
+          </form>
+          <hr/>
+          <form method="POST" data-action="{{ url_for('Airflow.clear') }}">
+            <input name="csrf_token" type="hidden" value="{{ csrf_token() }}"/>
+            <input name="dag_id" type="hidden" value="{{ dag.dag_id }}">
+            <input name="task_id" type="hidden">
+            <input name="execution_date" type="hidden">
+            <input name="origin" type="hidden" value="{{ request.base_url }}">
+            <div class="row">
+              <span class="btn-group col-md-9" data-toggle="buttons">
+                <label class="btn btn-default btn-sm" title="Also include past task instances when clearing this one">
+                  <input type="checkbox" value="true" name="past" autocomplete="off">
+                  Past
+                </label>
+                <label class="btn btn-default btn-sm" title="Also include future task instances when clearing this one">
+                  <input type="checkbox" value="true" name="future" autocomplete="off">
+                  Future
+                </label>
+                <label class="btn btn-default btn-sm" title="Also include upstream dependencies">
+                  <input type="checkbox" value="true" name="upstream" autocomplete="off">
+                  Upstream
+                </label>
+                <label class="btn btn-default btn-sm active" title="Also include downstream dependencies">
+                  <input type="checkbox" value="true" name="downstream" checked autocomplete="off">
+                  Downstream
+                </label>
+                <label class="btn btn-default btn-sm active">
+                  <input type="checkbox" value="true" name="recursive" checked autocomplete="off">
+                  Recursive
+                </label>
+                <label class="btn btn-default btn-sm" title="Only consider failed task instances when clearing this one">
+                  <input type="checkbox" value="true" name="only_failed" autocomplete="off">
+                  Failed
+                </label>
+              </span>
+              <span class="col-md-3">
+                <button id="btn_clear" type="submit" class="btn btn-primary btn-block"
+                    title="Clearing deletes the previous state of the task instance, allowing it to get re-triggered by the scheduler or a backfill command">
+                  Clear
+                </button>
+              </span>
+            </div>
+          </form>
+          <hr/>
+          <form method="POST" data-action="{{ url_for('Airflow.failed') }}">
+            <input name="csrf_token" type="hidden" value="{{ csrf_token() }}"/>
+            <input name="dag_id" type="hidden" value="{{ dag.dag_id }}">
+            <input name="task_id" type="hidden">
+            <input name="execution_date" type="hidden">
+            <input name="origin" type="hidden" value="{{ request.base_url }}">
+            <div class="row">
+              <span class="btn-group col-md-9" data-toggle="buttons">
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="failed_past" autocomplete="off">
+                  Past
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="failed_future" autocomplete="off">
+                  Future
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="failed_upstream" autocomplete="off">
+                  Upstream
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="failed_downstream" autocomplete="off">
+                  Downstream
+                </label>
+              </span>
+              <span class="col-md-3">
+                <button id="btn_failed" type="submit" class="btn btn-primary btn-block">
+                  Mark Failed
+                </button>
+              </span>
+            </div>
+          </form>
+          <hr/>
+          <form method="POST" data-action="{{ url_for('Airflow.success') }}">
+            <input name="csrf_token" type="hidden" value="{{ csrf_token() }}"/>
+            <input name="dag_id" type="hidden" value="{{ dag.dag_id }}">
+            <input name="task_id" type="hidden">
+            <input name="execution_date" type="hidden">
+            <input name="origin" type="hidden" value="{{ request.base_url }}">
+            <div class="row">
+              <span class="btn-group col-md-9" data-toggle="buttons">
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="success_past" autocomplete="off">
+                  Past
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="success_future" autocomplete="off">
+                  Future
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="success_upstream" autocomplete="off">
+                  Upstream
+                </label>
+                <label class="btn btn-default">
+                  <input type="checkbox" value="true" name="success_downstream" autocomplete="off">
+                  Downstream
+                </label>
+              </span>
+              <span class="col-md-3">
+                <button id="btn_success" type="submit" class="btn btn-primary btn-block">
+                  Mark Success
+                </button>
+              </span>
+            </div>
           </form>
           <hr/>
           <span class="btn-group" id="extra_links"></span>
         </div>
-
         <div class="modal-footer">
-          <button type="button" class="btn btn-default" data-dismiss="modal">
-            Close
-          </button>
+          <button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
         </div>
       </div>
     </div>
   </div>
   <!-- Modal for dag -->
-  <div class="modal fade" id="dagModal"
-        tabindex="-1" role="dialog"
-      aria-labelledby="dagModalLabel" aria-hidden="true">
+  <div class="modal fade" id="dagModal" tabindex="-1" role="dialog" aria-labelledby="dagModalLabel" aria-hidden="true">
     <div class="modal-dialog">
       <div class="modal-content">
         <div class="modal-header">
+          <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
           <h4 class="modal-title" id="dagModalLabel">
-            <span id='dag_id'></span>
+            <span class="text-muted">DAG:</span> {{dag.dag_id}}

Review comment:
       I need to make the "DAG" text dynamic as this modal can be used for "SUBDAG" too.




----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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


   Rebase on to latest master to fix test failures please


----------------------------------------------------------------
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 #10944: Task Instance Modal UX Enhancements

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



##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -108,199 +108,221 @@ <h4 class="pull-right">
   </div>
   <hr>
   <!-- Modal -->
-  <div class="modal fade" id="myModal"
-        tabindex="-1" role="dialog"
-      aria-labelledby="myModalLabel" aria-hidden="true">
+  <div class="modal fade" id="taskInstanceModal" tabindex="-1" role="dialog" aria-labelledby="taskInstanceModalLabel" aria-hidden="true">
     <div class="modal-dialog">
       <div class="modal-content">
         <div class="modal-header">
-          <h4 class="modal-title" id="myModalLabel">
-            <span id='task_id'></span>
-            <button id="btn_filter" type="button" class="btn btn-default" title="Filter on this task and upstream ">
-              <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
-            </button>
-              on
-            <span id='execution_date'></span>
+          <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
+          <h4 class="modal-title" id="taskInstanceModalLabel">
+            <span class="text-muted">Task Instance:</span> <span id="task_id"></span>
+            <br /><span class="text-muted">at:</span> <span id="execution_date"></span>
           </h4>
         </div>
         <div class="modal-body">
           <div id="div_btn_subdag">
-            <a id="btn_subdag" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.get_default_view()) }}">
+            <a id="btn_subdag" class="btn btn-primary" data-base-url="{{ url_for('Airflow.' + dag.get_default_view()) }}">
               Zoom into Sub DAG
             </a>
             <hr/>
           </div>
-          <a id="btn_task" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.task') }}">
+          <a id="btn_task" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.task') }}">
             Task Instance Details
           </a>
-          <a id="btn_rendered" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.rendered') }}">
+          <a id="btn_rendered" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.rendered') }}">
             Rendered
           </a>
-          <a id="btn_ti" type="button" class="btn btn-primary" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
+          <a id="btn_ti" class="btn btn-sm btn-default" data-base-url="{{ url_for('TaskInstanceModelView.list') }}">
             Task Instances
           </a>
-          <a id="btn_log" type="button" class="btn btn-primary" data-base-url="{{ url_for('Airflow.log') }}">
+          <a id="btn_log" class="btn btn-sm btn-default" data-base-url="{{ url_for('Airflow.log') }}">
             View Log
           </a>
+          <button id="btn_filter" type="button" class="btn btn-sm btn-default" title="Filter on this task and upstream ">
+            <span class="glyphicon glyphicon-filter" style="margin-left:0;" aria-hidden="true"></span>
+            Filter Upstream
+          </button>
           <hr/>
           <div id="dag_dl_logs">
-            <label style="display:inline"> Download Log (by attempts): </label>
+            <label style="display:inline"> Download Log (by attempts):</label>
             <ul class="nav nav-pills" role="tablist" id="try_index" style="display:inline">
             </ul>
             <hr/>
             <hr/>
           </div>
-          <div id="dag_redir_logs">
-            {% if external_log_name is defined %}
-              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts): </label>
+          {% if external_log_name is defined %}
+            <div id="dag_redir_logs">
+              <label style="display:inline"> View Logs in {{ external_log_name }} (by attempts):</label>
               <ul class="nav nav-pills" role="tablist" id="redir_log_try_index" style="display:inline">
               </ul>
               <hr/>
               <hr/>
-            {% endif %}
-          </div>
-          <form method="POST">
+            </div>
+          {% endif %}
+          <form method="POST" data-action="{{ url_for('Airflow.run') }}">

Review comment:
       The associated scripting below (line 555) is modifying form values priory to submitting, so it's not submitting to the `action` natively. The second to last line is basically changing `data-action` to `action`—this just felt like an explicit indicator when reading the code that this form is handled by scripting (not a vanilla `<form>`).
   ```js
       // Task Instance Modal actions
       $('form[data-action]').submit(function (e) {
         e.preventDefault();
         var form = $(this).get(0);
         form.execution_date.value = execution_date;
         form.origin.value = window.location;
         if (form.task_id) {
           form.task_id.value = task_id;
         }
         form.action = $(this).data("action");
         form.submit();
       })
   ```




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