feat: Add desktop version of theme page (fix #2559) #2568
Conversation
|
|
||
| const descriptionSanitized = sanitizeHTML( | ||
| nl2br(description), allowedDescriptionTags); | ||
| const summarySanitized = sanitizeHTML(addon.summary, ['a']); |
There was a problem hiding this comment.
Are these vars only used once? If so it might be nice to keep the use of the sanitization function closer to the dangerouslySetInnerHTML prop usage?
There was a problem hiding this comment.
The thought I was having is that it might make it easier to have a future lint rule that all content passed as HTML via dangerouslySetInnerHTML. should be wrapped in a sanitization function.
|
This can be a follow-up but I think there's some breakpoint tuning needed - it could probably stay 100% width longer see:
|
|
I'll see about the breakpoints or setting min-widths on those left sections really quickly, good catch. |
|
There's some small rtl issues at the similar width:
Not sure why the RTL debug lang chars are broken there (see the bottom) - maybe those glyphs don't exist in the font? |
There was a problem hiding this comment.
A couple of additional points. I wonder if there should be a max-width for the main content? On a really large screen the content get's so wide it's hard to read.
Another one to maybe follow-up on - should the read-more link be turned off on desktop?
Really looking nice!
r+wc
|
I wondered about content max-width but I say let the actual page expand as much as possible... max-width can also hide issues with display or make things hard to account for in my experience. But prose like the details should wrap after around 500px so I'll look into doing that in a follow-up. Read more like being off on desktop probably makes sense, although when we start adding more boxes underneath it might be useful; add-on descriptions can be wildly long and it will dominate that section of the page. |
|
Not sure why coveralls is complaining but coverage for those files is still at 100% and I didn't really touch much code even so merging... |
Adds support for themes on the desktop site and fixes issues from the last update.
This also improves the HTML and CSS used to create the same page for extensions (#2430).
Mobile
Desktop
-----------------