-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update Proposal list #1472
Update Proposal list #1472
Conversation
✅ Deploy Preview for fractal-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main entry to changes. Replaces current ActivityGovernance
component.
src/components/ui/badges/Badge.tsx
Outdated
const greenPlus2 = '#56A355'; | ||
const greenMinus5 = '#0A320A'; | ||
const greenHover = '#0E440E'; | ||
|
||
const redPlus4 = '#FFC7C7'; | ||
const redMinus2 = '#640E0D'; | ||
const redHover = '#76110F'; | ||
|
||
const sandBG = '#C18D5A'; | ||
const sandHover = '#B97F46' | ||
const blackText = '#150D04' | ||
|
||
const grayBG = '#9A979D' | ||
const grayHover = '#8C8990' | ||
|
||
const lightBlueBG = '#A3B9EC' | ||
const lightBlueHover = '#8DA8E7' | ||
const darkBlueText = '#0A1E3D' | ||
|
||
const darkBlueBG = '#1B3B83' | ||
const darkBlueHover = '#17326E' | ||
const lightBlueText = '#D1DCF5' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New colors are hardcoded in file for now. Not ideal I tried to use the name given in the figma file but some don't have colors yet. This can be move to design system once overhaul is done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uses meetsQuorum
to pull the information directly from contract for linearERC20 strategy token DAOs. I created a issue for update ERC721 contract to do the same.
if (truncate) { | ||
return ( | ||
<Text | ||
noOfLines={2} | ||
fontWeight={400} | ||
minWidth="100%" | ||
> | ||
{plainText} | ||
</Text> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this. Markdown looks pretty good. going straight to the markdown below. Let me know @mudrila if this is a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the idea was to remove styling applied by markdown in order to fit more actual content. I don't have strong opinion here but think we'd need some additional input to decide
src/i18n/locales/uk/common.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we still removing translations if the english json is updated?
Here is the current UI when Frozen. @xraystyle1980 @nicolaus-sherrill @xanaramoss There are few 'Activities' that have UI components but I haven't seen them in the UI in a while. This Frozen Activity is one of them. This PR doesn't touch the UI for these activities beyond the shared components (Badge, Proposal Title). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolaus-sherrill Coolio. updated colors and fixed alignment (which was a little tricky) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor suggestions, but other than that - looks good!
|
||
// @dev only azorius governance has quorum | ||
if ((proposal as SnapshotProposal).snapshotProposalId || !votingStrategy) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snapshot proposal also might have a quorum, yet it might be non-trivial to calculate it. Can you create a ticket so we'd investigate this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (truncate) { | ||
return ( | ||
<Text | ||
noOfLines={2} | ||
fontWeight={400} | ||
minWidth="100%" | ||
> | ||
{plainText} | ||
</Text> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the idea was to remove styling applied by markdown in order to fit more actual content. I don't have strong opinion here but think we'd need some additional input to decide
- ProposalCountdown updated for compadability
- removes other translation
@xraystyle1980 Colors are wrong? I'll take a look? I'll take another look at the padding as well. |
401d3b6
to
7eff319
Compare
@xraystyle1980 Ah I see ha. There were a few updates to the colors I didn't notice and didn't realize there was a hover text color. Ive updated the badge. I've adjusted the padding around the badge to 8px to match the designs more closely. |
Two line proposal title's force the proposer address to the wrong line see below proposal 14 (i.e. address below title) v.s proposal 13 where the address is on the same line |
My take is that keeping proposal titles to one line, and implementing some visual way to represent that there is more of the title would be best. However, if that's not reasonable/feasible, definitely want to see that author name on the same line as the proposal title. |
Yeah I actually ran into quite a bit of an issue getting these to play together with longer titles. Almost all the Proposals are fine except for the longer ones. |
Layout looks good @Da-Colon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making those pixel-perfect tweaks. Looks great!
Updated Proposal List (Dashboard, Proposals (Governance page), and updates Badge on Details
Testing:
For testing: Please view Dashboard, Proposals Page and Details of a few different DAO types. Make sure everything loads correctly. Here is the design link to compare. https://www.figma.com/file/dNWxEnscUonneTsNdzn6qZ/Components---Fractal-Design-System-0.0.1?type=design&node-id=10810-261&mode=design&t=qBGWJaHrBkbPIkTo-0
Here are some different DAO addresses:
0x259a5770d7242b5aBa7420926690E90c69037d10
0x70484d88cB248b9a5150d133fEDBa7948AfC9b0B
0xDD73b64C8845868CCddB30df83929fBA63b4d284
0x76DA0508ee8f900be06E58D8D56F418494C266d7
Screenshots:
Token DAO
Multisig DAO
Snapshot Proposals
Responsive Design
Details Page update (Badge | Proposal Title | Proposal Description)
Closes #1448