Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

ui: updated the ui of list items on deployments tab #2879

Merged
merged 7 commits into from
Jan 12, 2022

Conversation

sabrinako
Copy link
Contributor

@sabrinako sabrinako commented Jan 6, 2022

Notes

  • The URL shown on each list item is the deployment URL, the designs state it should be the release URL, but in consideration of the context and after discussion, we chose to show the deployment URL here
  • Currently the frontend does not receive the commit message with git commit information at all, so that needs to be implemented later
  • As noted in some other PRs, currently there are no animated icons in flight, so the Deploying... icon is the clock icon until the animated loading one is available

Screenshots

Note the left sidebar

Before

Screen Shot 2022-01-06 at 3 06 24 PM

After

Screen Shot 2022-01-12 at 12 33 14 PM

How to test

  1. Pull down the branch git checkout ui/deployment-card-state
  2. yarn start
  3. Check the deployments tab of an application
  4. Optional: test UI against a real Waypoint server

@github-actions github-actions bot added the ui label Jan 6, 2022
@github-actions
Copy link

github-actions bot commented Jan 6, 2022

Ember Asset Size action

As of 64d0ccb

Files that got Bigger 🚨:

File raw gzip
vendor.js +205 B +14 B
waypoint.js +8.57 kB +780 B
waypoint.css +756 B +234 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.css 0 B 0 B

@sabrinako sabrinako added this to the 0.7.0 milestone Jan 6, 2022
@sabrinako sabrinako linked an issue Jan 6, 2022 that may be closed by this pull request
@sabrinako sabrinako self-assigned this Jan 6, 2022
@sabrinako sabrinako marked this pull request as ready for review January 6, 2022 20:38
@sabrinako sabrinako requested review from almonk and a team January 6, 2022 20:38
@sabrinako sabrinako mentioned this pull request Jan 6, 2022
13 tasks
Copy link
Contributor

@jgwhite jgwhite left a comment

Choose a reason for hiding this comment

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

Looking good. I have one significant comment regarding accessibility of the health status indicator but all great apart from that.

Comment on lines 27 to 30
<span
data-test-health-status={{downcase vars.deploymentHealth}}
class="deploy-url__status deploy-url__status--{{downcase vars.deploymentHealth}}"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

CleanShot 2022-01-11 at 17 27 02@2x

When a URL is present, it looks like we’re only using the color of the indicator to convey the health status. We should add an additional means to convey this information.

Per WCAG SC 1.4.1:

Color is not used as the only visual means of conveying information, indicating an action, prompting a response, or distinguishing a visual element

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to hammer the point home, here’s how a simulation of how this would look to someone with deuteranopia (via Sim Daltonism):

CleanShot 2022-01-11 at 17 41 46@2x

Copy link
Contributor

Choose a reason for hiding this comment

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

@sabrinako How about we show the URL if and only if the health status is available? I think that would adhere pretty closely to the WCAG guidelines, on the basis that the presence of a URL implies availability.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gregone pointed out that we might want to also show the URL in the unknown state, for deployments that don’t/can’t provide health status. I think this is worth digging into, but perhaps in a subsequent PR.

ui/app/components/app-item/deployment.hbs Outdated Show resolved Hide resolved
ui/app/components/app-item/deployment.hbs Show resolved Hide resolved
@almonk
Copy link
Contributor

almonk commented Jan 12, 2022

I didn't make this clear enough so my bad – but my idea was to only show the release URL if the state was healthy. So in the case of any state OTHER than healthy, you'd see the status text...

CleanShot 2022-01-12 at 16 04 34@2x

Copy link
Contributor

@jgwhite jgwhite left a comment

Choose a reason for hiding this comment

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

🎉

@sabrinako sabrinako merged commit 62f10ca into main Jan 12, 2022
@sabrinako sabrinako deleted the ui/deployment-card-state branch January 12, 2022 19:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ui: convert deployment list info to new design
3 participants