fix: styles update #2126

joanna-pagepro · 2024-04-03T13:40:39Z

Description

There are a number of UI issues occurring across various motion widgets for all responsive sizing including desktop and mobile. These issues should be resolved to ensure they match Figma SOT and create a consistent and strong level of UX across the CDapp.

The issues should be tested and resolved across all responsive sizes unless stated.

Testing

Make any action using reputation method and go through steps

Resolves #2062

rdig

I'm good with the style / logic changes here, with the exception of the Tooltip component which really needs to be refactored, as we keep putting more and more props in it, and it's really not sustainable at this point.

rdig · 2024-04-05T13:40:06Z

src/components/shared/Extensions/Tooltip/Tooltip.tsx

@@ -11,6 +11,7 @@ const displayName = 'Extensions.Tooltip';
 const Tooltip: FC<PropsWithChildren<TooltipProps>> = ({
  children,
  tooltipContent,
+  tooltipStyle,


This is not the right approach here, as we are already passing in a className prop, that just sets classes on the wrapper. Plus we have isSuccess and isFullWidthContent which are just switches for yet more styles

The solution here is to refactor Tooltip to accept either themes, or the className prop to be more configurable and account for all the places we need to customize styles.

Otherwise, we'll just end up jamming in various props when we need to customize more things (like the arrow color for example)

I couldn't make it work using className, it had to be done by styles.

src/components/v5/common/ActionSidebar/partials/Motions/steps/OutcomeStep/hooks.ts

melyndav

Hey @joanna-pagepro - thanks for picking up my issue and great work so far.

A couple of pickups.

The Voting widget has empty space at the top

The percentage font is wrong in the outcome widget - see Figma below.

it's also impacting this component globally as you can see the dashboard font has also changed in the percentage bar -

The gray background of the percentage bar is wrong across all widgets

What it should be from Figma:

There is added spacing in some widgets but not in others between the value and the % symbol.
Currently:

As per Figma:

In the case where there are little users showing in the outcome widget, can we fill the empty space with the progress bar.

So currently, there is extra space to the right of the avatar

However, in Figma the avatars sit on the right and the progress bar fills the space.

Link to example: https://www.figma.com/file/l1dOM5qiQYwF0ElvKDqqjg/Design-System---Colony-v3?type=design&node-id=3134-9308&mode=design&t=fui5vBndDT4Ds1kj-4

ArmandoGraterol

Thank you for the changes! Left a small comment regarding the code, plus I still see that some of Mel's issues are still alive and kicking. Dunno if they are being handled separately.

src/components/v5/common/ActionSidebar/partials/Motions/steps/OutcomeStep/hooks.ts

melyndav

Hey @joanna-pagepro / @CzarekDryl,

Thanks for working on my feedback.

To clarify, there are needs to be two variations of the progress bar component with different % font sizes.

Uses the extra small font and is featured at the top of motion widgets. Marked in green.
Uses the small font and is used globally outside of motions in places like the dashboard objective widgets and in the main content of the motion widgets.

Currently, these keep being changed back and forth because when a change is requested when they need different fonts to match the context and placement.

This needs to be resolved across the top of all motion widgets where the percentage, vote numbers, reveal numbers etc is shown etc.

There is a padding issue above the main CTA inside the vote widget once a vote has been placed.

CzarekDryl · 2024-04-10T13:51:00Z

@ArmandoGraterol @melyndav Now everything should be fine

ArmandoGraterol

Looks good to me 👌 Thank you for the changes!

melyndav

Hey @CzarekDryl thanks for the further changes. Everything is looking correct now with the % font sizes.

However, the issue in the voting widget has popped up again, with extra space showing at the top of the widget.

CzarekDryl · 2024-04-12T10:40:59Z

@melyndav Margin fixed

amplify/backend/backend-config.json

melyndav

Hey @CzarekDryl thanks again for fixing the issues.

On mobile, all progress bars in the top information section should be full width of th widget and not match the text.

Here below, the progress bar/voting numbers in the reveal step isn't full width.

But in the voting widget in the previous step is full width

On mobile, the team reputation percentage data doesn't show in the widget

On the desktop version you data see it visible

On mobile again, when I confirmed the reveal stage it moved to the finalise step, but the top navigation doesn't move along with it to show the active step.

CzarekDryl · 2024-04-16T09:41:55Z

@melyndav Please check it once again :)

stale

ArmandoGraterol

Looking good! I've noticed the issues Sam and Mel pointed out are fixed for me. Thank you for the changes

ArmandoGraterol · 2024-04-25T19:46:46Z

src/components/v5/shared/Stepper/Stepper.tsx

+    if (!inView) {
+      setHiddenItem(name);
+    } else {
+      setHiddenItem(undefined);
+    }


Nitpick but we could make this simpler by doing this: setHiddenItem(!inView ? name : undefined)

iamsamgibbs

Thanks for the updates on this.

Timer padding is looking better on both mobile and desktop.

And tooltips on mobile are working fine too:

Only one final issue I can find. On certain breakpoints, when you reveal your vote, the 'finalize' pill won't be visible and nor will the arrows to scroll to it. If the 'active' pill is out of view, it should automatically scroll to that pill when that step becomes active.

Screen.Recording.2024-04-26.at.15.26.22.mov

CzarekDryl · 2024-05-07T13:27:33Z

@iamsamgibbs I can't reproduce this bug with no arrows on mobile.

iamsamgibbs · 2024-05-07T13:32:19Z

@iamsamgibbs I can't reproduce this bug with no arrows on mobile.

I think it specifically happens when revealing your vote as the additional pill for supported / rejected gets added.

CzarekDryl · 2024-05-27T10:13:48Z

@iamsamgibbs This should be fixed now.

iamsamgibbs

This looks good to me now! Thanks for the fix!

Screen.Recording.2024-05-29.at.19.36.49.mov

iamsamgibbs

Still looks good to me, hopefully this can get merged soon!

bassgeta

Almost there! Looking good, just noticed some things than can be improved.
When these are resolved, I think we are good to go!

src/components/v5/shared/Stepper/partials/StepperButton/StepperButton.tsx

src/components/v5/shared/ProgressBar/ProgressBar.tsx

bassgeta

Thank you for the changes, code looks good to me, styles too, nice work on this PR, now 🤞 we get it over the finish line!

iamsamgibbs

Still looking good!

melyndav

@CzarekDryl this one is a good to go from my end!! I do not see any more issues. I know this had been a very long process, but great work throughout it! 💯

joanna-pagepro assigned joanna-pagepro and CzarekDryl and unassigned joanna-pagepro Apr 3, 2024

CzarekDryl marked this pull request as ready for review April 5, 2024 10:16

CzarekDryl requested review from a team as code owners April 5, 2024 10:16

rdig previously requested changes Apr 5, 2024

View reviewed changes

ArmandoGraterol suggested changes Apr 5, 2024

View reviewed changes

src/components/v5/common/ActionSidebar/partials/Motions/steps/OutcomeStep/hooks.ts Outdated Show resolved Hide resolved

melyndav requested changes Apr 8, 2024

View reviewed changes

CzarekDryl requested review from ArmandoGraterol, melyndav and rdig April 8, 2024 11:07

ArmandoGraterol reviewed Apr 8, 2024

View reviewed changes

src/components/v5/common/ActionSidebar/partials/Motions/steps/OutcomeStep/hooks.ts Outdated Show resolved Hide resolved

melyndav requested changes Apr 9, 2024

View reviewed changes

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch from eedc0f6 to b1b53b6 Compare April 10, 2024 12:58

CzarekDryl requested review from melyndav and ArmandoGraterol April 10, 2024 13:51

ArmandoGraterol previously approved these changes Apr 10, 2024

View reviewed changes

melyndav requested changes Apr 11, 2024

View reviewed changes

CzarekDryl dismissed ArmandoGraterol’s stale review via 21043d2 April 12, 2024 07:51

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch from 72813b9 to 21043d2 Compare April 12, 2024 07:51

CzarekDryl requested review from melyndav and ArmandoGraterol April 12, 2024 10:41

ArmandoGraterol reviewed Apr 12, 2024

View reviewed changes

amplify/backend/backend-config.json Outdated Show resolved Hide resolved

melyndav requested changes Apr 15, 2024

View reviewed changes

CzarekDryl requested review from ArmandoGraterol and melyndav April 16, 2024 09:42

ArmandoGraterol previously approved these changes Apr 25, 2024

View reviewed changes

iamsamgibbs requested changes Apr 26, 2024

View reviewed changes

CzarekDryl dismissed ArmandoGraterol’s stale review via cfa5fab May 6, 2024 12:53

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch 2 times, most recently from cfa5fab to 6767240 Compare May 7, 2024 12:29

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch from 6767240 to 862106d Compare May 27, 2024 08:28

CzarekDryl requested a review from a team as a code owner May 27, 2024 08:28

CzarekDryl requested a review from iamsamgibbs May 27, 2024 10:13

iamsamgibbs previously approved these changes May 29, 2024

View reviewed changes

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch from 9eea0e7 to b0b5194 Compare June 11, 2024 10:41

fix: styles update

b67918d

CzarekDryl dismissed iamsamgibbs’s stale review via b67918d June 27, 2024 07:03

CzarekDryl force-pushed the fix/15840824-various-widget-ui-issues branch from b0b5194 to b67918d Compare June 27, 2024 07:03

fix: tooltip position

0eb10b0

iamsamgibbs previously approved these changes Jun 27, 2024

View reviewed changes

bassgeta requested changes Jul 1, 2024

View reviewed changes

src/components/v5/shared/Stepper/partials/StepperButton/StepperButton.tsx Outdated Show resolved Hide resolved

src/components/v5/shared/ProgressBar/ProgressBar.tsx Show resolved Hide resolved

fix: remove useEffect, progressLabel prop

359249c

CzarekDryl dismissed iamsamgibbs’s stale review via 359249c July 1, 2024 13:40

CzarekDryl requested review from bassgeta and iamsamgibbs July 1, 2024 13:41

bassgeta approved these changes Jul 1, 2024

View reviewed changes

iamsamgibbs approved these changes Jul 2, 2024

View reviewed changes

melyndav approved these changes Jul 3, 2024

View reviewed changes

CzarekDryl merged commit 4256fb6 into master Jul 3, 2024
4 of 6 checks passed

CzarekDryl deleted the fix/15840824-various-widget-ui-issues branch July 3, 2024 13:45

AdrianDudko mentioned this pull request Jul 22, 2024

Fix tooltips z-index in motions flow #2639

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: styles update #2126

fix: styles update #2126

joanna-pagepro commented Apr 3, 2024 •

edited by CzarekDryl

Loading

rdig left a comment

rdig Apr 5, 2024

CzarekDryl Apr 8, 2024

melyndav left a comment

ArmandoGraterol left a comment

melyndav left a comment

CzarekDryl commented Apr 10, 2024

ArmandoGraterol left a comment

melyndav left a comment

CzarekDryl commented Apr 12, 2024

melyndav left a comment

CzarekDryl commented Apr 16, 2024

ArmandoGraterol left a comment

ArmandoGraterol Apr 25, 2024

iamsamgibbs left a comment

CzarekDryl commented May 7, 2024

iamsamgibbs commented May 7, 2024

CzarekDryl commented May 27, 2024

iamsamgibbs left a comment

iamsamgibbs left a comment

bassgeta left a comment

bassgeta left a comment

iamsamgibbs left a comment

melyndav left a comment

fix: styles update #2126

fix: styles update #2126

Conversation

joanna-pagepro commented Apr 3, 2024 • edited by CzarekDryl Loading

Description

Testing

rdig left a comment

Choose a reason for hiding this comment

rdig Apr 5, 2024

Choose a reason for hiding this comment

CzarekDryl Apr 8, 2024

Choose a reason for hiding this comment

melyndav left a comment

Choose a reason for hiding this comment

ArmandoGraterol left a comment

Choose a reason for hiding this comment

melyndav left a comment

Choose a reason for hiding this comment

CzarekDryl commented Apr 10, 2024

ArmandoGraterol left a comment

Choose a reason for hiding this comment

melyndav left a comment

Choose a reason for hiding this comment

CzarekDryl commented Apr 12, 2024

melyndav left a comment

Choose a reason for hiding this comment

CzarekDryl commented Apr 16, 2024

ArmandoGraterol left a comment

Choose a reason for hiding this comment

ArmandoGraterol Apr 25, 2024

Choose a reason for hiding this comment

iamsamgibbs left a comment

Choose a reason for hiding this comment

CzarekDryl commented May 7, 2024

iamsamgibbs commented May 7, 2024

CzarekDryl commented May 27, 2024

iamsamgibbs left a comment

Choose a reason for hiding this comment

iamsamgibbs left a comment

Choose a reason for hiding this comment

bassgeta left a comment

Choose a reason for hiding this comment

bassgeta left a comment

Choose a reason for hiding this comment

iamsamgibbs left a comment

Choose a reason for hiding this comment

melyndav left a comment

Choose a reason for hiding this comment

joanna-pagepro commented Apr 3, 2024 •

edited by CzarekDryl

Loading