#297 Validate abi params entry when adding it #312
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
@nevendyulgerov, I think the issue would be more around not allowing to save if it is in a state of error rather than not allowing declarations to be added. As an example, is that with that change I am not allowed to either declare a Struct (a.k.a tuple) first or declare an abi parameter signature that uses a tuple ahead of time (i.e. before I declare the tuple) I am limited to inline declarations. My suggestion is to rollback that one and focus on not allowing the save to go through when in an error state, as it does when the name is empty.
New changes, I am not able to define a struct.
Current production deployed. After using a Struct ahead of time, then declaring it.
…w entry. Display props error only when the entries are empty. Memoize entries error.
| @@ -86,7 +89,7 @@ export const HumanReadableABIParameter: FC<HumanReadableABIParameter> = ( | |||
| Add | |||
| </Button> | |||
| } | |||
| error={props.error || form.errors.abiParamEntry} | |||
| error={isNilOrEmpty(entries) ? props.error : null} | |||
There was a problem hiding this comment.
Hi @nevendyulgerov, this here is doing two different things. The original code was saying that if the caller of that component is setting an error, it has higher precedence than an internal error. However, the change only displays the external set error (if any) if the entries are nil/empty.
There was a problem hiding this comment.
Hi, @brunomenezes , looking at the code there's no validation for abiParamEntry so the second part of the expression will always evaluate to null, so in any case only the external error (coming from props.error) can be displayed here.
The reason for this change was that without it, there will be 2 errors for the abi params displayed when users attempt to save an ABI Params spec with invalid params.
Here's a video demonstrating this, using error={props.error || form.errors.abiParamEntry}:
https://github.com/user-attachments/assets/01a949d7-1ed8-49fc-be37-cf7c47d202bf
Another video, using error={isNilOrEmpty(entries) ? props.error : null}:
https://github.com/user-attachments/assets/79549060-f210-440d-bee5-48a51981bbac
So my idea here is to display the external error only when no entries were added, because specAbiParamValidation verifies that as well. specAbiParamValidation produces validation errors only in this case when no entries were added. In the case when there are entries and some are invalid (when validating with parseAbiParameters), the error doesn't need to be displayed underneath the abi params input, as there will be already another error displayed under the added entry card, below the input.
I hope this makes sense :) Please let me know if this approach is ok.
There was a problem hiding this comment.
What about edit mode? I think it is relevant in that context.
I tweaked slightly the logic for adding abi params from the specs form. Now, when clicking on the
Addbutton next to the abi params input, we'll first parse the params to ensure they're valid. Only if they are, we'll add them to the entries array.I kept the validation for entries, this line:
so that users can still see an error for the entries if they previously saved a spec with invalid params.