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

Landing cta #933

Merged
merged 7 commits into from
Sep 24, 2018
Merged

Landing cta #933

merged 7 commits into from
Sep 24, 2018

Conversation

jannekoivistoinen
Copy link
Contributor

No description provided.

@@ -0,0 +1,2 @@
REACT_APP_USE_EXAMPLE_CUSTOM_ATTRIBUTES=true
REACT_APP_GOOGLE_MAPS_API_KEY=AIzaSyDZhsJGsRv0c9HykUHfnVAIVOb1tWtqrTg
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is in wrong place.
You should write these variables to the file called .env

@@ -104,7 +104,7 @@
font-weight: var(--fontWeightBold);
font-size: 30px;
line-height: 36px;
letter-spacing: -0.5px;
letter-spacing: -1px;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be mentioned in the Changelog.
If one of our customer has changed the font (or is just happy with current letter-spacing), it might need some adjustment from their own code base.
- marketplaceH1FontStyles: changed letter spacing to be more tight.

buttons elements should have zero padding */
padding: 20px 0 0 0;
buttons elements should have zero padding */
padding: 16px 0 0 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is likely to break buttons and baselines on every page.
Let's not do it on marketplace.css level!
SectionHero.css is better less scary place to make this change.

@@ -212,6 +212,7 @@

@media (--viewportMedium) {
min-height: 65px;
padding: 20px 0 0 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is related to the comment above

This is likely to break buttons and baselines on every page.
Let's not do it on marketplace.css level!
SectionHero.css is better less scary place to make this change."

name="SearchPage"
to={{
search:
's?address=Finland&bounds=70.0922932%2C31.5870999%2C59.693623%2C20.456500199999937',
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably tell about this in Change log.
- SectionHero has now a search page link that should be customized to point to your marketplace primary area

Copy link
Contributor

@Gnito Gnito left a comment

Choose a reason for hiding this comment

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

Check comments.
(I can help with those change requests)

@jannekoivistoinen jannekoivistoinen merged commit d37c90f into master Sep 24, 2018
@jannekoivistoinen jannekoivistoinen deleted the landing-cta branch September 24, 2018 13:11
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.

2 participants