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

ui: added sidebar/panel ui to deployments tab #2773

Merged
merged 2 commits into from
Dec 1, 2021
Merged

Conversation

sabrinako
Copy link
Contributor

@sabrinako sabrinako commented Nov 23, 2021

Screenshot

Screen Shot 2021-11-23 at 3 31 37 PM

Notes

  • Build + release tabs are actually going to be in a different table format, so this PR just changes the deployments tab
  • Changes to paths
    • The sidebar view that outlets to the details page is now the deployment route/template
    • The details page is the deployment/deployment-seq route/template
    • deployments route exists just to redirect to the deployment route
    • There's obviously some changes, but a lot of the code was just relocated
    • StatusReportMetaTable alignment is fixed (but this could have been from a previous PR and I just didn't notice)

How to test

  1. Pull down this branch git checkout ui/sidebar-panel
  2. yarn (new ember-route-helpers dependency)
  3. yarn start
  4. Navigate to the deployments tab (should automatically load when you select an application)
  5. Make sure everything is working as expected

@github-actions
Copy link

github-actions bot commented Nov 23, 2021

Ember Asset Size action

As of 6f9f3bb

Files that got Bigger 🚨:

File raw gzip
vendor.js +7.03 kB +942 B
waypoint.js +4.45 kB +677 B
waypoint.css +548 B +241 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.css 0 B 0 B

@sabrinako sabrinako changed the title ui: added sidebar/panel ui to deployments tab WIP ui: added sidebar/panel ui to deployments tab Nov 23, 2021
@sabrinako sabrinako self-assigned this Nov 23, 2021
@sabrinako sabrinako added this to the 0.7.0 milestone Nov 23, 2021
@sabrinako sabrinako changed the title WIP ui: added sidebar/panel ui to deployments tab ui: added sidebar/panel ui to deployments tab Nov 23, 2021
@sabrinako sabrinako requested a review from a team November 23, 2021 21:36
@sabrinako sabrinako linked an issue Nov 23, 2021 that may be closed by this pull request
@jgwhite jgwhite requested a review from almonk November 24, 2021 09:46
@jgwhite
Copy link
Contributor

jgwhite commented Nov 24, 2021

I wonder if we can boost the z-index for this state, so the outline doesn’t go below other items:

CleanShot 2021-11-24 at 10 48 44@2x

@jgwhite
Copy link
Contributor

jgwhite commented Nov 24, 2021

Empty state seems fine to me, but noting for other reviewers:

localhost_4200_default_empty_app_web_deployment

@almonk
Copy link
Contributor

almonk commented Nov 24, 2021

Nice to see this coming to life! There's some work to do to make this sparkle. Here's some thoughts below... For styling issues I'm always happy to pair on resolving them – just let me know.

General styling

I've outlined some issues with the styling below.
Frame 218

List items in the overview component don't sit at equal distance from each other:
CleanShot 2021-11-24 at 10 31 52

As an side, I'm seeing some very odd resizing behaviour (probably unrelated to this PR)....

Screen.Recording.2021-11-24.at.10.24.14.mov

Deployment list info

In the designs I removed all the platform info and replaced it with commit info. I know this won't always be available but maybe we should have a whiteboarding session on how this component should behave given varying levels of data.

CleanShot 2021-11-24 at 10 26 22

If we want to do that work in a different PR thats totally fine with me – we can instead focus on polishing what we have right now.

Further design required

I'll have a think about how we can represent mutable deployments in a way that works with the reduced horizontal space. Right now this is too busy to be easily parseable I think.

CleanShot 2021-11-24 at 10 33 26

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 so good! I’ve made a few superficial comments, but certainly no structural changes needed.

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.

Changes all look great. From my perspective, this looks ready to merge.

Copy link
Contributor

@gregone gregone 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 a comment about what I think is a UI bug (see below), but I don't think it's related.

@gregone
Copy link
Contributor

gregone commented Nov 25, 2021

@sabrinako Possibly not related:
Screen Shot 2021-11-25 at 15 36 42

On the builds page, the build time badges are flowing below the item. I believe this could be because we remove the display: flex on app-item

@gregone
Copy link
Contributor

gregone commented Nov 25, 2021

A few more comments: cc @almonk
Screen Shot 2021-11-25 at 15 47 41

@sabrinako
Copy link
Contributor Author

sabrinako commented Nov 29, 2021

Comments Addressed (not including comments addresssed before 11/29)

This is mainly for me but also transparency on what might be pushed to future work

@almonk

  • Badge styling
  • Gray well styling (logs + resources table)
  • Text truncation (and title added to link element)
    • Sidebar
    • Resources table
  • List items in Overview equal distance
  • Resizing behavior
  • Deployment list info (LATER, needs to be discussed)
  • Mutable deployment list info (LATER, needs to be discussed)

@jgwhite

  • Dark mode selected list item
  • Fix focus ring hidden on list item

@gregone

  • Empty state height as vh
  • Scrollable list
  • Display destroyed deployments button match design

@jgwhite
Copy link
Contributor

jgwhite commented Nov 30, 2021

Cool to see Percy playing its part on this PR! And it looks like it may have actually spotted some unintended behavior:

CleanShot 2021-11-30 at 11 03 40@2x

@gregone
Copy link
Contributor

gregone commented Nov 30, 2021

Sorry for being misleading, as discussed in Zoom, let's drop the vh change

Co-authored-by: Jamie White <[email protected]>
Co-authored by: Greg Hoin <[email protected]>
@sabrinako sabrinako merged commit ca015e6 into main Dec 1, 2021
@sabrinako sabrinako deleted the ui/sidebar-panel branch December 1, 2021 19:21
@jgwhite
Copy link
Contributor

jgwhite commented Dec 1, 2021

🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants