Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Disable spellchecker in eth address field - re #2257 #2557

Merged
merged 3 commits into from
Jul 21, 2021

Conversation

gabriel-horvat
Copy link
Contributor

What it solves

Browser was underlining the input text with a red wavy line. But an address isn't supposed to be spell-checked.

Environment

Browser: Chrome

How this PR fixes it

Disabled HTML spellcheck for safe address input element.

Screenshots

Unfixed:
unfixed

Fixed:

Screen Shot 2021-07-19 at 3 28 20 PM

@github-actions
Copy link

CLA Assistant Lite:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger the CLA Action by commenting recheckcla in this Pull Request

@gabriel-horvat
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

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

Awesome, thank you @gabriel-horvat!

@gabriel-horvat
Copy link
Contributor Author

gabriel-horvat commented Jul 19, 2021

For sure! Am loving this project!
Do you have a suggestion for a more involved contribution @katspaugh ?
Or should I just pick whichever one seems interesting from the backlog?

@katspaugh
Copy link
Member

recheckcla

@@ -203,7 +203,7 @@ const SafeOwnersForm = (props): React.ReactElement => {
testId={`create-safe-owner-name-field-${index}`}
/>
</Col>
<Col className={classes.ownerAddress} xs={7}>
<Col className={classes.ownerAddress} spellcheck="false" xs={7}>
Copy link
Member

@mmv08 mmv08 Jul 19, 2021

Choose a reason for hiding this comment

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

Col is a div element, it's not an input

You should place the attribute to the generic AddressInput component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally, you're so right! Thank you @mikheevm

@katspaugh
Copy link
Member

@gabriel-horvat if you’d like to work on a complete new feature, #970 might be interesting. Otherwise, yeah, whatever strikes your fancy, we’ll be happy to help.

@gabriel-horvat
Copy link
Contributor Author

Thank you @katspaugh, will take a look at #970 later today

@katspaugh katspaugh added this to Ready for QA in Gnosis Safe React (Archived) Jul 19, 2021
Comment on lines 227 to 229
type="text"
autoCorrect={false}
spellCheck={false}
Copy link
Member

@mmv08 mmv08 Jul 20, 2021

Choose a reason for hiding this comment

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

You have already set the spellCheck attribute on the generic AddressInput component, you do not need to repeat it here. If you want to add a different attribute (autoCorrect), please add it to the generic component

Also, please pay attention to the types: in the component, you pass false as a string but here you pass it as a boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it!

Copy link
Member

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

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

Thank you!

@dasanra dasanra linked an issue Jul 20, 2021 that may be closed by this pull request
@dasanra dasanra removed this from Ready for QA in Gnosis Safe React (Archived) Jul 20, 2021
@francovenica
Copy link
Contributor

This PR is not creating an env to check the fix.
@mikheevm @dasanra can you see why is failing to create the env?

@mmv08
Copy link
Member

mmv08 commented Jul 21, 2021

@francovenica honestly I have no idea, it looks like it cannot get some environment variables because the PR is from a forked repository. I would say this PR is fine to be merged without QA approval because it doesn't touch the logic at all

@francovenica
Copy link
Contributor

@mikheevm Ok, if it looks fine in the code for you then let's merge. I'll check it once is on dev

@dasanra dasanra merged commit fd22f6e into 5afe:dev Jul 21, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable the spellchecker in the address field
5 participants