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

feat: Show hints on proposal editor to get verified or to get turbo #773

Closed
wants to merge 8 commits into from

Conversation

ChaituVR
Copy link
Member

@ChaituVR ChaituVR commented Sep 16, 2024

Test only after merging snapshot-labs/snapshot-hub#925 and snapshot-labs/snapshot-hub#926

Summary

Closes: https://github.com/snapshot-labs/workflow/issues/134

  • Show a message when someone tries to post a proposal over the limit
  • Show a message when someone tries to add more than the choices limit
  • Show a message when someone tries to add more than the body chars limit
  • Fix in apps/ui/src/helpers/validation.ts - Throws an error if we try to add more choices than the limit while keeping all choices empty

How to test

  1. Update limits in apps/ui/src/helpers/turbo.ts
    for example:
diff --git a/apps/ui/src/helpers/turbo.ts b/apps/ui/src/helpers/turbo.ts
index 18869ff9..57f3753f 100644
--- a/apps/ui/src/helpers/turbo.ts
+++ b/apps/ui/src/helpers/turbo.ts
@@ -1,15 +1,15 @@
 export const MAX_BODY_LENGTH = {
-  default: 10000,
+  default: 1,
   turbo: 40000
 };
 
 export const MAX_CHOICES = {
-  default: 500,
+  default: 5,
   turbo: 1000
 };
 
 export const MAX_1D_PROPOSALS = {
-  default: 20,
+  default: 1,
   turbo: 40
 };

When body chars limit is reached:
image

When proposals limit is reached:
image

When choices limit is reached:
image

Copy link

changeset-bot bot commented Sep 16, 2024

⚠️ No Changeset found

Latest commit: cce78bb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ChaituVR ChaituVR marked this pull request as draft September 16, 2024 12:11
@ChaituVR ChaituVR marked this pull request as ready for review September 16, 2024 12:27
@wa0x6e
Copy link
Contributor

wa0x6e commented Sep 16, 2024

Text and the left fire icon do not seem to be aligned

};

export const MAX_CHOICES = {
default: 500,
Copy link
Member

Choose a reason for hiding this comment

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

Can you change default to be 10 for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? to test?
try running this in the console

for (let i = 0; i < 500; i++) {
  document.querySelector(".static .s-base.mb-5 button.rounded-full.border.button.text-skin-link.bg-skin-bg.w-full.flex.items-center.justify-center.space-x-1").click();
}

apps/ui/src/helpers/turbo.ts Outdated Show resolved Hide resolved
@bonustrack
Copy link
Member

The icon should be aligned the the text and not appear in the center of 2 lines.
image

Copy link
Contributor

@wa0x6e wa0x6e left a comment

Choose a reason for hiding this comment

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

Beside the few UI issues raised above, everything's working well for me

Daily proposal limit on unverified spaces should be on another PR I think, since not related to turbo
There's also the issue about choices count above limit not showing any error message, but not related to this PR

@ChaituVR
Copy link
Member Author

ChaituVR commented Sep 20, 2024

If proposal limit is reached and If space is not verified:
image

If proposal limit is reached and If space is not turbo but verified:
image

If choice limit is reached and If space is not turbo:
image

if chars limit is reached and If space is not turbo: (char limit is 1 here, just to get screenshot) - Mobile
image

@ChaituVR ChaituVR changed the title feat: Show hints for turbo feat: Show hints on proposal editor to get verified or to get turbo Sep 20, 2024
Copy link
Contributor

@wa0x6e wa0x6e left a comment

Choose a reason for hiding this comment

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

tAck

@wa0x6e
Copy link
Contributor

wa0x6e commented Sep 21, 2024

We're currently showing this hint for onchain spaces, when proposal body is too long (> 10K).

Do not make sense since turbo is not supported yet for onchain spaces

@bonustrack
Copy link
Member

Sorry I'm changing the design, I played a bit with it locally and figured out that the error and the turbo hint are saying something too similar that it doesn't worth making it separate, instead I propose something more light like this:
image

Others comments:
A: Spacing between the "Add choice" and error message should be 2.
B: It should use leading-6 as line-height
C: We should show formated number with ,

For space proposal limit you can do like this:
image
image

@ChaituVR
Copy link
Member Author

Continued in #803

@ChaituVR ChaituVR closed this Sep 23, 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.

3 participants