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

2.0 UI Overhaul/Refresh #11195

Merged
merged 15 commits into from
Oct 2, 2020
Merged

2.0 UI Overhaul/Refresh #11195

merged 15 commits into from
Oct 2, 2020

Conversation

ryanahamilton
Copy link
Contributor

@ryanahamilton ryanahamilton commented Sep 29, 2020

Resolves #10953.

A refreshed UI for the 2.0 release. The existing "theming" is a bit long in the tooth and this PR attempts to give it a modern look and some freshness to compliment all of the new features under the hood.

The majority of the changes to UI have been done through updates to the Bootstrap theme contained in bootstrap-theme.css. These are simply overrides to the default stylings that are packaged with Bootstrap.

I've tweaked, refactored, and cleaned up syntax across most of the view, script, and CSS files.

I've highlighted outstanding questions in bold below.

Color

Implements a full refresh of the color palette based on the colors in the brand book (outlined in black below). Those colors have been expanded to a full color system robust enough to cover all aspects of the UI.

Where would the most ideal place to document this? Out of scope for this PR, but I would typically put these all into CSS variables in a shared import file as documentation.

image

Iconography

The current icon system (Bootstrap Glyficons) has been replaced with the Material Design Icons (Apache license version 2.0). They have a clean, modern look, and a robust selection. Like Glyficons, It is implemented via webfont. I attempted (and would have preferred) to install it via NPM package, but it appears that it is not kept up-to-date with their CDN/downloadable libraries, so instead am serving it via static files. The font has been converted to a Base64 encoding and embedded within the new material-icons.css stylesheet. This yields a cacheable file that doesn't require multiple requests (of importing font files).

Most of the icons are very similar to their predecessors, others I've attempted to select the best representation possible based on the available set.
Image 2020-09-29 at 2 32 16 PM

Typography

The UI currently does not utilize either of the font-families (Rubik, Operator Mono) designated by the brand guidelines. I did some experimentation with these, but there were some immediate, glaring problems in regards to the overall widths of Rubik. I may follow-up with a partial implementation that only applies to headings.

Global Header

  • Updated the global nav menus to open on hover instead of just on click.
  • Removed icons from the global nav items and their children (in drop-downs). They were implemented inconsistently and used the AppBuilder supplied FontAwesome icons. This simplifies the presentation and prevents us from mixing multiple conflicting-styled icon systems.
  • Removed the "About" navigation menu. It only contained a link to the "Version" view. Instead, I've taken the contents of the Version view and added a global footer that displays the same information.
  • Simplified the Time Zone selector by removing the day and seconds. The hours are the most relevant part of the timestamp when it comes to timezones. The constant motion of the changing seconds felt like a bit of a distraction. Tweaked the styling within the dropdown as well.
  • Replaced the current user's name with a circled monogram derived from their initials. Displays an icon fallback if first and last name values not present.
  • I've partially removed the ability to define a custom background color of the navbar (navbar_color), it likely wouldn't translate well for users who currently utilizing it since the navbar was previously optimized for a dark background bar. Appears to be a bit of a superfluous feature. Is it something we can remove completely in order to ensure a clean presentation, or does the support need to be left in? (edit: retained this feature)
  • I threw a little interaction easter egg in there—can you spot it?

Before:
image

After:
image

Fallback icon when no first/last name present:
Image 2020-09-29 at 1 35 21 PM

Version info in global footer:
image

DAGs View (Home)

  • Updated the table presentation with a cleaner look that should be a bit more pleasing to look at.
  • Actions have been separated from links to better communicate their purpose. I do question wether the links should be here at all. They add a lot of visual noise and are certainly not intuitive for new users.
Before After
image image

DAG Views

Most notable change across these views is the cleaned up run selection form fields. They now have a tidy presentation that is consistent across the views. I believe this further highlights the incongruence of the Task Instance navigation and why I've proposed #11089.

Before:

image

After:

image

Flask AppBuilder CRUD Views

These views are automatically generated by AppBuilder, so there isn't access to markup/icon modifications (hence why they still use the Font Awesome icons). It is reliant on Bootstrap markup/classes, so I was able to update the colors to utilize the new theming.

image

Misc.

Added all non-vendor stylesheets to the Webpack configuration. This outputs them with a fingerprinted filename and ensures the latest version isn't missed by the client due to caching.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Sep 29, 2020
@ryanahamilton
Copy link
Contributor Author

Waiting until I've addressed any forthcoming feedback before cutting new screenshots for the Docs.

@ashb ashb requested review from ashb, kaxil, potiuk and turbaszek and removed request for ashb September 29, 2020 16:04
@ashb
Copy link
Member

ashb commented Sep 29, 2020

First and Last name appear to be required fields—are there any cases where there might not be values and need a fallback?

When using various auth backends, either of these could be an empty string, which I think is "valid" according to the DB schema.

Is it something we can remove completely in order to ensure a clean presentation, or does the support need to be left in?

This is often used by some users to give multiple environments a distinct colour ("green for staging, red for production" etc.)

@ashb
Copy link
Member

ashb commented Sep 29, 2020

One comment on the DAG view:

The "running" dag is visually placed on the top right, but it is a property of which dag run is currently being selected. I'm not sure if this is a problem where you have it or not

@ryanahamilton
Copy link
Contributor Author

@ashb:

  1. Added a fallback icon when first/last name are not present. See updated PR description for screenshot.
  2. I added the navbar_color capability back in.
  3. I know what you mean about the "running" placement as it was moved from graph.html to dag.html template. It still only displays when on the Graph view—it didn't appear to cause any issues.

Copy link
Member

@feluelle feluelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Graph View, what does "Runs" actually do?
I mean isn't there always only one run displayed regardless of what you select in "Runs"?

And do we really need the "Update" button or can we update on changed setting?

@feluelle
Copy link
Member

Nvm "Runs" adds the runs you can select from in "Run" correct?

@ryanahamilton
Copy link
Contributor Author

And do we really need the "Update" button or can we update on changed setting?

@feluelle I agree with your suggestion, though that's a refactor for another PR 😉

@ausiddiqui
Copy link

This looks great! Had a few requests:

Is it possible to add a legend in the main DAGs View (Home) for the Recent Tasks? There are many columns and colors and hard to keep track of. This might also be handled through a tooltip on the circle itself.

When carrying out any action, such as "Mark as failed" or "Mark as success" a new page appears to confirm the action. The two action buttons are labeled "OK!" and "bail." in v1.10.10 (non RBAC UI). Can these labels be changed to just say "OK"/"Yes"/"Continue" and "Cancel"?

In much of the UI timestamps are exhibited like "09-16T13:45:08.842496+00:00" would there be a way to alter this format universally or in specific pages through config values. Execution times can be used as identifiers, but visually not sure the value of the milliseconds down to 6 digits of precision.

@WSHoekstra
Copy link
Contributor

I love most of it, but there are a few things I'd like to see differently.

  • I'll use the 'Search Dags' bar more than the 'Search by tags' search bar, so I'd like to see their positions reversed
  • the existence of the 'Delete DAG' button makes it impossible to grant semi-technical users access to the UI (e.g. stakeholders who sometimes want to rerun relevant ETL's at will). Would it be possible to either hide the button altogether (after all - proper dag bag configuration should be done in code) or have an option in the admin settings to disable it there?

@ryw ryw self-requested a review September 30, 2020 15:12
Copy link
Member

@ryw ryw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this @ryanahamilton - with 2.0 alpha/beta testing we'll all be able to exercise the UI changes, and we can continue to refine the aesthetics - as long as everything works technically, I support merging this.

@kaxil
Copy link
Member

kaxil commented Oct 1, 2020

I love most of it, but there are a few things I'd like to see differently.

  • I'll use the 'Search Dags' bar more than the 'Search by tags' search bar, so I'd like to see their positions reversed
  • the existence of the 'Delete DAG' button makes it impossible to grant semi-technical users access to the UI (e.g. stakeholders who sometimes want to rerun relevant ETL's at will). Would it be possible to either hide the button altogether (after all - proper dag bag configuration should be done in code) or have an option in the admin settings to disable it there?

The Delete DAG button won't be an issue coz from Airflow 2.0 RBAC UI would be the default UI and you can give DELETE permissions to only admins for example

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I prefer to keep "whitespace"/formatting only changes to their own PR/commit (so it'as easier to see in git blame what is formatting only, and what is functional.)

Not strictly required in this case though.

@ashb
Copy link
Member

ashb commented Oct 1, 2020

@ryanahamilton Look slike your version change isn't working. Tests are failing with:

E   jinja2.exceptions.UndefinedError: 'airflow_version' is undefined

@ashb
Copy link
Member

ashb commented Oct 1, 2020

Could you show a "full" page screenshot showing the global footer in place please?

Additionally: if the git version is not set (which I think is fairly common) we should just not show it in the HTML, rather than showing N/A

@ryanahamilton
Copy link
Contributor Author

@ashb re: footer:

  • Test failures have been resolved (there were a couple more than the one you spotted)
  • I fixed issue where the airflow_version and git_version version weren't globally available (in FAB CRUD views).
  • Updated the footer to only show to authenticated users as to not reveal version info
  • Git version is no longer shown when there is no value
  • Full screenshot of the footer:

Image 2020-10-01 at 11 13 07 AM

Updated Docs screenshots still to-do

@kaxil kaxil merged commit 24d0ecf into apache:master Oct 2, 2020
@kaxil kaxil deleted the ui_refresh branch October 2, 2020 14:59
ashb added a commit to astronomer/airflow that referenced this pull request Oct 7, 2020
As part of apache#11195 we re-styled the UI, changing a lot of the default
colours to make them look more modern. However for anyone upgrading and
keeping their airflow.cfg from 1.10 to 2.0 they would end up with things
looking a bit ugly, as the old navbar color would be kept.

This uses the existing config value upgrade feature to automatically
change the old default colour in to the new default colour.
ashb added a commit that referenced this pull request Oct 7, 2020
As part of #11195 we re-styled the UI, changing a lot of the default
colours to make them look more modern. However for anyone upgrading and
keeping their airflow.cfg from 1.10 to 2.0 they would end up with things
looking a bit ugly, as the old navbar color would be kept.

This uses the existing config value upgrade feature to automatically
change the old default colour in to the new default colour.
@ryanahamilton ryanahamilton added the area:UI Related to UI/UX. For Frontend Developers. label Nov 18, 2020
@himabindu07
Copy link

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

@blag blag mentioned this pull request Feb 8, 2022
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.

Refresh Airflow UI