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

Theme update #198

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Theme update #198

wants to merge 7 commits into from

Conversation

shannonwells
Copy link
Collaborator

@shannonwells shannonwells commented Nov 23, 2024

Goal

The goal of this PR is to pull in the Style Guide and use it where possible, and otherwise make the style match what is on Frequency.xyz and other branded sites.

Known bug: after changing networks the path is appended and not cleared first. One result is the Nav indicator for the current page disappears. See #197

Not included: updates to styles, given we do not have mocks for the new style for this site.

Closes #195

Checklist

  • PR Self-Review and Commenting
  • Tests updated

How To Test the Changes

git fetch && git checkout theme-update
npm run dev
  1. Click through steps to: connect to network, connect a provider, start to Add an Account Id or Stake, change networks, Become a Provider
  2. Make sure the style is consistent and all links are visible, modals can be canceled, etc.

@shannonwells shannonwells marked this pull request as ready for review November 27, 2024 18:38
@@ -14,7 +14,7 @@

<button
Copy link
Contributor

Choose a reason for hiding this comment

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

The FAQ container is not centered on the screen.
Screenshot 2024-11-27 at 11 16 17 AM

@@ -14,7 +14,7 @@

<button
id="faq-question"
class="label text-md mt-2 flex w-full items-center justify-between border-t border-divider p-4 text-left"
class="label border-divider mt-2 flex w-full items-center justify-between border-t p-4 text-left text-md"
Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, it doesn't seem to match the style guide. Either import the one from the SG, or abide by the styles here: https://www.figma.com/design/EjFt1XdacOogDZoQ7RetrI/Frequency-Style-Guidelines?node-id=1072-1096&node-type=frame&t=M9rvMiiDpjs2zIh5-0

Copy link
Contributor

@claireolmstead claireolmstead left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-27 at 11 24 29 AM The "Provider Dashboard" seems pretty far away from the header. Screenshot 2024-11-27 at 11 25 15 AM The dropdown's don't abide by the styleguide either. I'm not seeing the placeholder text, or the disabled state, and the icon looks very small and pushed to the right.

Copy link
Contributor

@claireolmstead claireolmstead left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-27 at 11 26 59 AM If we're going to change the `Provider Login` width, we should also change the `Become a Provider` with to match. They should all be using a reusable component and be centered on the screen.

Copy link
Contributor

@claireolmstead claireolmstead left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-27 at 11 29 54 AM The spacing is too tight (I don't think there is any) in tablet or smaller desktop sizes.

@shannonwells shannonwells marked this pull request as draft November 27, 2024 21:03
@claireolmstead
Copy link
Contributor

Screen.Recording.2024-12-03.at.12.31.00.PM.mov

Doesn't look like the button is disabled on hover.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

A couple of places I think need improvements but can be done in other stories.

  • There should be a spinner or loading period when trying to connect to a network. Currently there is no user feedback.
  • There is still a hover state on drop downs when they should be disabled.
  • We're still displaying the Questions button in the header of the FAQs page. Seems redundant.
  • If you go to "Become a provider" then reload, the active state on the nav bar disappears.
  • The size of the elements in the footer don't match the size in the SG.
  • The footer does not stick to the bottom when content heights are less than viewport heights.

@claireolmstead
Copy link
Contributor

Screenshot 2024-12-03 at 12 55 36 PM This component looks like it lost the spacing between the underline and body text.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 00 17 PM

This hover state is not defined anywhere. The text changing color to the teal is more standard.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 01 56 PM

We lost spacing here

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 02 32 PM

The input and dropdown should have consistent styling.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screen.Recording.2024-12-03.at.1.03.43.PM.mov

The copy button is white now so you cant see it. change to black or navy. Also needs to be centered.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 08 21 PM The Become a Provider transaction succeeded but we are no longer automatically being logged in when we should be.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 09 41 PM
  • There is a lot of extra spacing here that wasn't there before.
  • This modal also used to have the current network and accountId selected on open.
  • Button should be disabled unless there is a new account and network selected.

@claireolmstead
Copy link
Contributor

claireolmstead commented Dec 3, 2024

Screenshot 2024-12-03 at 1 12 06 PM

Spacing does not seem right here either.

@claireolmstead
Copy link
Contributor

Screenshot 2024-12-03 at 1 12 26 PM Spacing does not seem right here either.

@claireolmstead
Copy link
Contributor

Screenshot 2024-12-03 at 1 12 54 PM When navigating to Activity Log, the side nav doesn't change.

@claireolmstead
Copy link
Contributor

Screen.Recording.2024-12-03.at.1.13.51.PM.mov

The switch SVG in the upper right is not rendering correctly in all pages.

@claireolmstead
Copy link
Contributor

Screenshot 2024-12-03 at 1 14 57 PM FAQ font sizes don't align with the styleguide. https://www.figma.com/design/EjFt1XdacOogDZoQ7RetrI/Frequency-Style-Guidelines?node-id=1072-1096&node-type=frame&t=llKhFYOQ6Fu0XvWe-0

@claireolmstead
Copy link
Contributor

Screen.Recording.2024-12-03.at.1.15.52.PM.mov

There is no longer a hover state on the nav bar.

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.

Update Provider Dashboard Styles and Links
2 participants