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

UX Fix: Prevent undesired text selection with DAG title selection in Chrome #8912

Merged
merged 2 commits into from
May 19, 2020

Conversation

ryanahamilton
Copy link
Contributor

Prevent undesired text selection (in Chrome browser) when DAG title is double-click selected.

Simply utilizes user-select: none; CSS on the label to prevent its selection.

Behavior before:
Screen Recording 2020-05-19 at 08 45 AM

Behavior after:
Screen Recording 2020-05-19 at 08 46 AM


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label May 19, 2020
@ashb
Copy link
Member

ashb commented May 19, 2020

I guess that means that the schedule is now not selectable at all? Hmmm. Stupid Chrome making us make decisions.

@ryanahamilton
Copy link
Contributor Author

@ashb yeah, it's a trade-off. The schedule link is still clickable/functional. The DAG title certainly seems like the more likely element for a user to want to select.

@ashb
Copy link
Member

ashb commented May 19, 2020

Is it worth doing user-select:none; -moz-user-select: auto so it stays working in FF, since this is a chrome only bug?

@ryanahamilton
Copy link
Contributor Author

@ashb that's a good idea… I tend to avoid vendor prefixes when possible, but in this case we'd be utilizing it to undo a hack. The user-select: none doesn't affect Safari, so the behavior is already as desired there. Will update.

@ashb ashb changed the title UX Fix: Prevent undesired text selection with DAG title selection UX Fix: Prevent undesired text selection with DAG title selection in Chrome May 19, 2020
@ashb ashb merged commit ce7fdea into apache:master May 19, 2020
@ashb ashb added this to the Airflow 1.10.11 milestone May 19, 2020
@ashb ashb deleted the chrome_text_select branch May 20, 2020 09:51
kaxil pushed a commit that referenced this pull request Jun 22, 2020
…Chrome (#8912)

Negate user-select in Firefox where behavior is already as desired

(cherry-picked from ce7fdea)
potiuk pushed a commit that referenced this pull request Jun 29, 2020
…Chrome (#8912)

Negate user-select in Firefox where behavior is already as desired

(cherry-picked from ce7fdea)
kaxil pushed a commit that referenced this pull request Jul 1, 2020
…Chrome (#8912)

Negate user-select in Firefox where behavior is already as desired

(cherry-picked from ce7fdea)
@ryanahamilton ryanahamilton added the area:UI Related to UI/UX. For Frontend Developers. label Nov 18, 2020
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
…Chrome (apache#8912)

Negate user-select in Firefox where behavior is already as desired

(cherry-picked from ce7fdea)
@loociano
Copy link

An alternative fix that does not use user-select:none; -moz-user-select: auto; would be to remove the <div> that wraps the headings used for DAG name and schedule.

Before:

<div>
  <h3> ...DAG name code... </h3>
  <h4> ...Schedule code... </h4>
<div>
<div class="clearfix"></div>

After:

<h3> ...DAG name code... </h3>
<h4> ...Schedule code... </h4>
<div class="clearfix"></div>

@ashb
Copy link
Member

ashb commented Jul 14, 2021

@loociano PR welcome 😁

@loociano
Copy link

loociano commented Jul 14, 2021

I was first wondering why this issue is still occurring a year later.
Despite #8912 being listed in 1.10.11, @ryanahamilton's fix is not included in the release code (not even in 1.10.15). Am I missing something?

@ashb
Copy link
Member

ashb commented Jul 14, 2021

@loociano 1663760 is the commit that is included in 1.10.11

@loociano
Copy link

@loociano 1663760 is the commit that is included in 1.10.11

My bad, I was looking into airflow/www/templates/airflow/dag.html, however the fix is in airflow/www_rbac/templates/airflow/dag.html

@ashb do you know the difference between www and www_rbac? Should the fix be included in both?

@ashb
Copy link
Member

ashb commented Jul 14, 2021

In 1.10, www is the classic/deprecated UI, and hasn't been recieving fixes for a while (since ~1.10.6), www_rbac is what becomes the www in 2.0, and can be enabled on 1.10 releases by setting rbac = True in the airflow.cfg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants