Frontend for contact page #545
Conversation
|
Deployment previews on netlify for branch
|
rosalieper
force-pushed
the
contactPage
branch
2 times, most recently
from
3ff4f5e
to
f16beb3
Compare
link the contact page on the home page and the footer. Added mock backend api for testing. Bug: T318400
rosalieper
force-pushed
the
contactPage
branch
from
f16beb3
to
7afaeeb
Compare
|
Generally looks good to me from a code perspective. I added a thought to the ticket about which key the recaptcha token is sent in. Right now this is not in the key specified in the ticket but it might be that we want to change the specification not the code. One thing that is notable is that this doesn't implement some error state. We should probably add that. (see https://phabricator.wikimedia.org/T318400#8428059). |
|
Generally the functionality looks like the mock. The only thing I notice is missing (other than the above error handling) is length limits on the other fields and "required" validation. I think we "agreed" 300 chars would be sensible for these other fields. Probably best not to show a counter on these though :) |
|
@tarrow Charlie asked for a counter as I remember? |
|
so I think the message field should have both the validation and the counter. I think the other fields should have the validation (for both being "required" if the are and for the max length) but no counter |
| errorMessage: false | ||
| }), | ||
| methods: { | ||
| closeAlert () { |
There was a problem hiding this comment.
This method will close the snackbar in both case (success and error). I don't know if it'll be better if I have 2 separate methods for each case? @tarrow
There was a problem hiding this comment.
I think there isn't a massive motivation to go in either direction. Right now I think this is fine :)
|
Quite some time on: I think this now meets with the requirements and closely matches figma. I believe that the precise levels of the headings in figma do not match, however in order to visually replicate the figma docs this current version seems to make sense. I shall approve the PR but will hold off on merging until the "sister" PR for the backend has successfully made it to production. |
|
Look like we want to chase up any missing padding bits once this makes it to the users :) |
No description provided.