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/12/08 15:09:43 UTC
[GitHub] [airflow] ryanahamilton commented on a change in pull request #12911: Disable Pause/Unpause switch if user doesn't have edit access for DAG.
ryanahamilton commented on a change in pull request #12911:
URL: https://github.com/apache/airflow/pull/12911#discussion_r538404496
##########
File path: airflow/www/static/css/switch.css
##########
@@ -49,6 +49,10 @@
cursor: pointer;
}
+.disabled {
+ cursor: auto;
+}
+
Review comment:
Instead of adding another class we should leverage the `disabled` attribute. We can also make the disabled state a bit more obvious by altering the opacity. Add the following right after the `.switch::before` selector:
```css
.input:disabled + .switch {
opacity: 0.4;
cursor: not-allowed;
}
```
##########
File path: airflow/www/templates/airflow/dag.html
##########
@@ -40,15 +40,26 @@
<span class="material-icons" aria-hidden="true">keyboard_arrow_up</span>
DAG: {{ dag.parent_dag.dag_id }}</a>
{% endif %}
+
<div>
<h3 class="pull-left">
{% if dag.parent_dag is defined and dag.parent_dag %}
<span class="text-muted">SUBDAG:</span> {{ dag.dag_id }}
{% else %}
- <label class="switch-label js-tooltip" title="Pause/Unpause DAG">
- <input class="switch-input" id="pause_resume" data-dag-id="{{ dag.dag_id }}" type="checkbox" {{ "checked" if not dag.is_paused else "" }}>
- <span class="switch" aria-hidden="true"></span>
- </label>
+ {% if appbuilder.sm.can_edit_dag(dag.dag_id) %}
+ <label class="switch-label js-tooltip" title="Pause/Unpause DAG">
+ <input class="switch-input" id="pause_resume" data-dag-id="{{ dag.dag_id }}"
+ type="checkbox" {{ "checked" if not dag.is_paused else "" }}>
+ <span class="switch" aria-hidden="true"></span>
+ </label>
+ {% else %}
+ <label class="switch-label js-tooltip disabled" title="DAG is {{ 'Paused' if dag.is_paused else 'Active' }}">
+ <input class="switch-input" id="toggle-{{ dag.dag_id }}" data-dag-id="{{ dag.dag_id }}"
+ type="checkbox" {{ "checked" if not dag.is_paused else "" }} disabled>
+ <span class="switch disabled" aria-hidden="true"></span>
+ </label>
+ {% endif %}
+
Review comment:
Can keep these simple with just a single set of markup? Paired with my CSS suggestion, a single `disabled` attribute on the `<input>` is all that needs to change.
Would need some extra logic (example below) for the tooltip to change. Alternatively, we could just disable the tooltips too (by removing the `js-tooltip` class).
```suggestion
{% if appbuilder.sm.can_edit_dag(dag.dag_id) %}
{% set switch_tooltip = 'Pause/Unpause DAG' %}
{% else %}
{% set switch_tooltip = 'DAG is Paused' if dag.is_paused else 'DAG is Active' %}
{% endif %}
<label class="switch-label js-tooltip" title="{{ switch_tooltip }}">
<input class="switch-input" id="pause_resume" data-dag-id="{{ dag.dag_id }}" type="checkbox"{{ " checked" if not dag.is_paused else "" }}{{ " disabled" if not appbuilder.sm.can_edit_dag(dag.dag_id) else "" }}>
<span class="switch" aria-hidden="true"></span>
</label>
```
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -103,9 +103,17 @@ <h2>DAGs</h2>
<tr>
{# Column 1: Turn dag on/off #}
<td style="padding-right:0;">
- <label class="switch-label js-tooltip" title="Pause/Unpause DAG">
- <input class="switch-input" id="toggle-{{ dag.dag_id }}" data-dag-id="{{ dag.dag_id }}" type="checkbox" {{ "checked" if not dag.is_paused else "" }}>
- <span class="switch" aria-hidden="true"></span>
+ {% if dag.can_edit %}
+ <label class="switch-label js-tooltip" title="Pause/Unpause DAG">
+ <input class="switch-input" id="toggle-{{ dag.dag_id }}" data-dag-id="{{ dag.dag_id }}"
+ type="checkbox" {{ "checked" if not dag.is_paused else "" }} >
+ <span class="switch {{ '' if dag.can_edit else 'disabled' }}" aria-hidden="true"></span>
+ {% else %}
+ <label class="switch-label js-tooltip disabled" title="DAG is {{ 'Paused' if dag.is_paused else 'Active' }}">
+ <input class="switch-input" id="toggle-{{ dag.dag_id }}" data-dag-id="{{ dag.dag_id }}"
+ type="checkbox" {{ "checked" if not dag.is_paused else "" }} disabled>
+ <span class="switch disabled" aria-hidden="true"></span>
+ {% endif %}
Review comment:
Similar suggestions here.
```suggestion
{% if dag.can_edit %}
{% set switch_tooltip = 'Pause/Unpause DAG' %}
{% else %}
{% set switch_tooltip = 'DAG is Paused' if dag.is_paused else 'DAG is Active' %}
{% endif %}
<label class="switch-label js-tooltip" title="{{ switch_tooltip }}">
<input class="switch-input" id="pause_resume" data-dag-id="{{ dag.dag_id }}" type="checkbox"{{ " checked" if not dag.is_paused else "" }}{{ " disabled" if not dag.can_edit else "" }}>
<span class="switch" aria-hidden="true"></span>
</label>
```
----------------------------------------------------------------
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