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

chore: Update Desktop GUI footer design #8702

Merged
merged 10 commits into from Oct 6, 2020
Merged

Conversation

chrisbreiding
Copy link
Contributor

@chrisbreiding chrisbreiding commented Sep 30, 2020

User facing changelog

Miscellaneous: Updated the design of the test runner GUI's footer. It now shows an indicator if there is a new version available.

Additional Information

Changelog link query param forwarding relies on this PR being deployed: https://github.com/cypress-io/cypress-services/pull/3105

How has the user experience changed?

Before (on latest version)

Screen Shot 2020-09-30 at 12 01 09 PM

Before (update available)

Screen Shot 2020-09-30 at 12 01 59 PM

After (on latest version)

Screen Shot 2020-09-30 at 11 59 36 AM

After (update available)

Screen Shot 2020-10-01 at 10 39 34 AM

PR Tasks

  • Have tests been added/updated?
  • N/A Has the original issue been tagged with a release in ZenHub?
  • N/A Has a PR for user-facing changes been opened in cypress-documentation?
  • N/A Have API changes been updated in the type definitions?
  • N/A Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 30, 2020

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Sep 30, 2020



Test summary

3830 0 55 1


Run details

Project cypress
Status Passed
Commit b7c95d5
Started Oct 6, 2020 4:23 AM
Ended Oct 6, 2020 4:34 AM
Duration 11:10 💡
OS Linux Debian - 10.2
Browser Firefox 77

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@chrisbreiding chrisbreiding marked this pull request as ready for review September 30, 2020 16:04
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Where is the original design for this? Didn’t see it linked in TR-352. Nevermind, the Figma doesn't show up on my phone for some reason.

From what I see on the design, the 'Update Available' blue banner should no longer show when there is an update. The only way to update is clicking the version button with the arrow.

The changelog link is supposed to have this query param: ?source=dgui_footer I think this requires it being it’s own on link?? I can’t remember if the query param will pass along correctly if it’s just https://on.cypress.io/changelog ?source=dgui_footer

I didn't pull down the code to run so this will require further review after these changes are made.

@chrisbreiding
Copy link
Contributor Author

From what I see on the design, the 'Update Available' blue banner should no longer show when there is an update. The only way to update is clicking the version button with the arrow.

I'm not sure this is the case (or should be the case). We're changing the style of the update banner to be a dismiss-able toast in a later story, but I'm not sure what the plan is in the interim since this could potentially get released before then. I'll check with Adam about it.

The changelog link is supposed to have this query param: ?source=dgui_footer I think this requires it being it’s own on link?? I can’t remember if the query param will pass along correctly if it’s just https://on.cypress.io/changelog ?source=dgui_footer

Thanks for pointing this out. I didn't mess with the existing link since TR-352 says "The changelog should continue linking to ...", so I thought there was no change there, but I guess it is different. I'll look into how on links handle query params.

@chrisbreiding
Copy link
Contributor Author

chrisbreiding commented Oct 1, 2020

In regards to on links and query params, looks like it's necessary to specify in the on manifest links.yml for a particular redirect to forwardQueryParams, so I'll submit a PR to do that for changelog links.

@chrisbreiding
Copy link
Contributor Author

Confirmed with Adam this is supposed to remove the update banner altogether.

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

  1. Is the hover of the changelog meant to have the margin to the right of it like this? Seems a bit weird, thought it would extend to the edge. It doesn't seem totally clear from the comp. (maybe I'm missing some way for Figma to be interactive here). @CypressAdam

    Screen Shot 2020-10-02 at 2 21 34 PM
  2. After clicking the update button, the styling of the button still appears 'focused' (You can see this in the tests also).

  3. Can we add some percy snapshots around these tests? Cause this stuff is hard to catch if it's broken later.

@CLAassistant
Copy link

CLAassistant commented Oct 2, 2020

CLA assistant check
All committers have signed the CLA.

@CypressAdam
Copy link
Contributor

@jennifer-shehane I nixed the padding-right on .footer to clear up that gap. I was playing in a dream world in Figma where I had corresponding margins in the header. It now matches up.

@chrisbreiding hope you don't mind my taking the liberty. 2 and 3 from Jennifer's list are beyond my capabilities 🧑‍🚀

@jennifer-shehane jennifer-shehane merged commit 4f5be35 into develop Oct 6, 2020
@Saibamen
Copy link
Contributor

Saibamen commented Oct 6, 2020

IMHO just icon is not enough.

I will suggest bringing back text version, and use new footer (with icon), like here:
image

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 14, 2020

Released in 5.4.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v5.4.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Oct 14, 2020
@chrisbreiding chrisbreiding deleted the tr-352-footer-update branch April 5, 2022 18:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants