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

Reskin Moves ProposalCountdown from the Badge component #1786

Merged

Conversation

Da-Colon
Copy link
Contributor

Separate the ProposalCountdown from the Badge component

image
image
image

Fixes #1695

@Da-Colon Da-Colon requested review from adamgall, DarksightKellar, mudrila and a team May 14, 2024 19:53
@Da-Colon Da-Colon self-assigned this May 14, 2024
@Da-Colon Da-Colon linked an issue May 14, 2024 that may be closed by this pull request
Copy link

netlify bot commented May 14, 2024

Deploy Preview for fractal-dev ready!

Name Link
🔨 Latest commit c5bc25b
🔍 Latest deploy log https://app.netlify.com/sites/fractal-dev/deploys/664401ddf1b90a0008726aca
😎 Deploy Preview https://deploy-preview-1786.app.dev.fractalframework.xyz
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

I think you can cleanup the props to ProposalCountdown a bit.

At first glance, textStyle will _alwaysbelabel-base`, so can probably remove it and inline the style.

The showIcon prop is only true in one usage of this component... Do we really need it?

Etc.

@Da-Colon Da-Colon force-pushed the reskin-1695-countdown-timer-width branch from 36c0e50 to 2ad58aa Compare May 15, 2024 00:25
@Da-Colon
Copy link
Contributor Author

I think you can cleanup the props to ProposalCountdown a bit.

At first glance, textStyle will _alwaysbelabel-base`, so can probably remove it and inline the style.

The showIcon prop is only true in one usage of this component... Do we really need it?

Etc.

Updated to remove the textStyle. But the other two props textColor and showIcon are both being used.

These defaults seem to mainly be for the the TxActions component for the Timelock on the Button.

Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

Let's remove icons logic pls

Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

:shipit:

@Da-Colon Da-Colon requested a review from adamgall May 15, 2024 14:22
@adamgall adamgall changed the title Moves ProposalCountdown from the Badge component Reskin Moves ProposalCountdown from the Badge component May 15, 2024
@adamgall
Copy link
Member

To @Da-Colon's point, The "logo" discussion doesn't have anything to do with countdown timer for active proposal and the "Active" chip. sorry for derailing this PR.

@tomstuart123
Copy link

tested for both vote period and timelock. both work really well

Thanks @Da-Colon

(note I did find an issue but not related to this PR)

@adamgall adamgall merged commit 5f25eba into reskin/refactor-root-reskin May 15, 2024
7 checks passed
@adamgall adamgall deleted the reskin-1695-countdown-timer-width branch May 15, 2024 16:52
@adamgall adamgall mentioned this pull request May 15, 2024
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.

Countdown timer width
5 participants