Themes: added Support tab to sheet #5282
Conversation
1. Support page layout done 2. Moved tab code to individual render calls 3. Added footer line component
folletto
added
the
[Feature Group] Appearance & Themes
|
Added:
Props @seear for the cherries ;) |
|
Fixed:
The reason was that IE11 with a flex width of 100% forces the item to go beyond the container boundaries if the flex property for |
folletto
added
[Status] Needs Review
|
We may want to open the forum links in a fresh browser tab, since they are not part of Calypso. |
|
I wondered about that... While true in terms of our rule of thumb, I couldn't come up with a scenario where the user intent clicking there would be happy to have a second tab open. The button also seems quite a clear indication of a separate navigation flow. |
| }, | ||
|
|
||
| renderOverviewTab() { | ||
| return <div> |
There was a problem hiding this comment.
Multiline markup blocks tend to go inside (), see renderFeaturesCard below.
There was a problem hiding this comment.
* facepalm * fixed.
|
Very minor—iPad width has the text right up against the forum button: Maybe the buttons need some margin? |
|
I'd say this is good to merge when you are happy 👍 |
|
Ok, fixed:
|
folletto
removed
the
[Status] Needs Review
| @@ -9,6 +9,7 @@ | |||
| import React from 'react'; | |||
| import { connect } from 'react-redux'; | |||
| import page from 'page'; | |||
| import { isPremium } from 'my-sites/themes/helpers'; | |||
There was a problem hiding this comment.
Not an external dependency :-) Plus we're also importing from that file here -- so that line should be changed to
import { isPremium, getForumUrl } from 'my-sites/themes/helpers';(And https://github.com/Automattic/wp-calypso/pull/5282/files#diff-f576b3c13daa17f9ccee514c4c46bc07R177 needs to be changed accordingly.)
There was a problem hiding this comment.
You can cherry-pick 51ef28fced3fe489deeca4c17f0ea35d9023cea5 for a fix :-p
There was a problem hiding this comment.
Ah good point. Since it's already merged tho we can just review that PR and merge it, right? :)
There was a problem hiding this comment.
Yeah this will be fixed by #5259.
import { isPremium, getForumUrl } from 'my-sites/themes/helpers';
Incidentally, the above doesn't work since getForumUrl uses this.isPremium().
There was a problem hiding this comment.
Incidentally, the above doesn't work since getForumUrl uses this.isPremium().
Mhm, noticed last night. Going to ES6ify helpers, probably as part of #5284.
This PR does:
Still Todo
The correct URL for the Theme ForumA visual glitch on the mobile size on IE11.To test