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

UI adjustments for sky #619

Merged
merged 12 commits into from
Sep 5, 2024
Merged

UI adjustments for sky #619

merged 12 commits into from
Sep 5, 2024

Conversation

DaeunYoon
Copy link
Contributor

@DaeunYoon DaeunYoon commented Sep 3, 2024

Make UI adjustments to the UI to align with the new brand.
(color theme, logo)

Checklist:

  • issue number linked above after pound (#)
    • replace "Closes " with "Contributes to" or other if this PR does not close the issue
  • issue checkboxes are all addressed
  • manually checked my feature / not applicable
  • wrote tests / not applicable
  • attached screenshots / not applicable
image image image

@DaeunYoon DaeunYoon marked this pull request as ready for review September 3, 2024 10:34
Copy link
Contributor

@LukSteib LukSteib left a comment

Choose a reason for hiding this comment

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

  • NIT: On hover of this primary button the button caption becomes barely readable (see s1). Can we change that in a way that it's still readable (e.g. white)

s1
Screenshot 2024-09-03 at 16 56 06

@DaeunYoon
Copy link
Contributor Author

image image

fyi: we need to update preview image that is used for twitter card later on as well :)

LukSteib
LukSteib previously approved these changes Sep 4, 2024
frontend/components/auction/TransactionMessage.vue Outdated Show resolved Hide resolved
frontend/assets/icons/logo.svg Outdated Show resolved Hide resolved
frontend/assets/images/background-image.png Outdated Show resolved Hide resolved
frontend/components/layout/Header.vue Outdated Show resolved Hide resolved
frontend/components/layout/LandingBlock.vue Outdated Show resolved Hide resolved
@@ -12,7 +11,6 @@
:is-wallet-loading="isWalletLoading"
:has-accepted-terms="hasAcceptedTerms"
:staging-banner-url="stagingBannerURL"
:production-banner-url="productionBannerURL"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this back, as this is out of scope of this update. This banner is being used to differentiate staging from production, see it in action here: https://main.auctions-ui.k8s.sidestream.tech

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the variable back, but not sure StagingBanner uses productionBannerURL.
I removed it with banner that refers to the electron app which was outlined in the issue. Do you think we need to bring this one back as well?

Copy link
Contributor

@valiafetisov valiafetisov Sep 5, 2024

Choose a reason for hiding this comment

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

Ah, maybe it's just a confusing name of the variable (as well as confusing use of it). This can be tested by providing PRODUCTION_BANNER_URL env variable to the frontend

frontend/components/layout/Header.vue Outdated Show resolved Hide resolved
@DaeunYoon
Copy link
Contributor Author

DaeunYoon commented Sep 4, 2024

Screenshots image image

I used mono icon for mobile-screen size (testing with web browser, the logo wasn't rendered properly on smaller screen)

(tested on chrome)
image

image

@valiafetisov
Copy link
Contributor

Did some minor tweaks to the colors based on the Brand Guideline.

Screenshots Screenshot 2024-09-04 at 21 18 16 Screenshot 2024-09-04 at 21 18 23 Screenshot 2024-09-04 at 21 21 16

valiafetisov
valiafetisov previously approved these changes Sep 4, 2024
Copy link
Contributor

@LukSteib LukSteib left a comment

Choose a reason for hiding this comment

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

Sorry for NIT picking:

  • ensure that font in dropdown for network and wallet has other color than black (e.g. white) because currently hard to read

s1
image

@DaeunYoon
Copy link
Contributor Author

image

@valiafetisov valiafetisov merged commit 20a99bc into main Sep 5, 2024
8 checks passed
@valiafetisov valiafetisov deleted the ui-adjustments-for-sky branch September 5, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants