-
Notifications
You must be signed in to change notification settings - Fork 400
feat: Add desktop home page #2655
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
Conversation
f00c093
to
c300190
Compare
I wonder if the medium layout should the heading and explantion text aligned to the top of the module rather than vertically centered? |
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.
r+wc
</h2> | ||
<p className="Home-description"> | ||
{i18n.gettext( | ||
'Install powerful tools that make browsing faster and safer, add-ons make your browser yours.')} |
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.
You can use a un-tagged template literal here if that helps with wrapping.
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.
Oh good call, will do.
display: block; | ||
} | ||
|
||
span { |
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.
Might be nice to move this below the other link styles that come after this.
} | ||
} | ||
|
||
.Home-extensions-link:link::before { |
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.
E_LINK_OVERLOAD
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.
Not that there's an obvious alternative afaict.
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.
Yeah I really forget why we're using :link
on a
styles but my goodness is it frustrating.
const fakeDispatch = sinon.stub(); | ||
|
||
return shallow( | ||
<HomeBase |
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.
Other DIY stores are available
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.
Hahaha I thought that too. (For those not in the UK)
); | ||
} | ||
|
||
if (clientApp === CLIENT_APP_FIREFOX) { |
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.
Would be nice to have these work from the API at some point but probably better to do that after cats are consolidated.
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.
Yeah, the thing is the category API just returns everything... I guess we could request filters on it though.
const { store } = dispatchSignInActions(); | ||
|
||
const root = findRenderedComponentWithType(renderIntoDocument( | ||
<Provider store={store}> |
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.
Shallow ftw
c300190
to
98f572f
Compare
98f572f
to
fa90fc1
Compare
Fix mozilla/addons#10473.
Converts the existing home content to work on desktop... we'll get carousels and such in the future because this was just about porting existing mobile stuff over. But hey–we did it. On the last day. If someone reviews this 😉 😅
Mobile
Medium
Large