-
Notifications
You must be signed in to change notification settings - Fork 14
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
[MPT-63] Adding call to action to apply to homepage #730
Conversation
✅ Deploy Preview for tender-meitner-99286b 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.
This PR is generally a good start, although I think we may need to adjust some of the surrounding margins between the testimonial carousel and the footer.
Please set up the Husky hooks as follows, for your code to be autoformatted upon committing:
pages/index.tsx
Outdated
<a | ||
target="_blank" | ||
rel="noopener noreferrer" | ||
href="https://advisorysg.typeform.com/to/NQaJmE6j#source=mentorsite" |
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.
Could this direct to /apply
instead (without any of the other properties, e.g. target
or rel
)?
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.
Made the change as requested. Button links directly to /apply
pages/index.tsx
Outdated
rel="noopener noreferrer" | ||
href="https://advisorysg.typeform.com/to/NQaJmE6j#source=mentorsite" | ||
> | ||
<h1 style={{ color: "black" }}>Apply Now</h1> |
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.
Although the font size should be big, it isn't ideal to set this as a h1
since that is semantically intended for text headings. Perhaps you can use Tailwind styles to increase the font size?
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.
Made the changes.
styles/App.css
Outdated
@@ -10,6 +10,7 @@ | |||
--darker-gray-color: #1a1a1a; | |||
--black-color: #000; | |||
--green-color: #28a745; | |||
--light-orange-color:#fedf81; |
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.
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.
For now I have removed the background colour. I have used the standard brand colour for the call to action.
@wei2912 as discussed, are we changing the link of the apply now button to link to the application form instead of having a separate page? |
Co-authored-by: Tan Wee Joe <[email protected]>
Merged in 4a94fc9. |
Added call to action section to homepage.