Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29491561: Issue 5413 - Create basic style guide for website-defaults (Closed)

Created:
July 18, 2017, 6:53 a.m. by ire
Modified:
Aug. 22, 2017, 4:58 p.m.
Reviewers:
juliandoucette
Visibility:
Public.

Description

Issue 5413 - Create basic style guide for website-defaults

Patch Set 1 #

Total comments: 3

Patch Set 2 : More basic styleguide #

Total comments: 1

Patch Set 3 : Apply patch against CMS compatible website-defaults #

Total comments: 22

Patch Set 4 : Remove styleguide specific styling, remove advanced elements #

Patch Set 5 : Use includes for each demo #

Total comments: 16

Patch Set 6 : Formatting fixes #

Patch Set 7 : Separate Content and Developer Styleguides #

Unified diffs Side-by-side diffs Delta from patch set Stats (+286 lines, -82 lines) Patch
A includes/styleguide/grid/2-column.html View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
A includes/styleguide/grid/3-column.html View 1 2 3 4 5 6 1 chunk +10 lines, -0 lines 0 comments Download
A includes/styleguide/grid/4-column.html View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
A includes/styleguide/grid/all-column-widths.html View 1 2 3 4 5 6 1 chunk +13 lines, -0 lines 0 comments Download
A includes/styleguide/grid/index.html View 1 2 3 4 5 6 1 chunk +16 lines, -0 lines 0 comments Download
A includes/styleguide/lists/description.html View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
A includes/styleguide/lists/index.html View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
A includes/styleguide/lists/ordered.html View 1 2 3 4 5 6 1 chunk +14 lines, -0 lines 0 comments Download
A includes/styleguide/lists/unordered.html View 1 2 3 4 5 6 1 chunk +14 lines, -0 lines 0 comments Download
A includes/styleguide/typography/blockquotes.html View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
A includes/styleguide/typography/body.html View 1 2 3 4 5 6 1 chunk +21 lines, -0 lines 0 comments Download
A includes/styleguide/typography/headings.html View 1 2 3 4 5 6 1 chunk +12 lines, -0 lines 0 comments Download
A includes/styleguide/typography/index.html View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
M pages/content.html View 1 2 3 4 1 chunk +5 lines, -39 lines 0 comments Download
A pages/content-styleguide.md View 1 2 3 4 5 6 1 chunk +81 lines, -0 lines 0 comments Download
A pages/developer-styleguide.html View 1 2 3 4 5 6 1 chunk +36 lines, -0 lines 0 comments Download
M pages/grid.html View 1 2 3 4 5 1 chunk +8 lines, -43 lines 0 comments Download
M pages/index.html View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 24
ire
July 18, 2017, 6:53 a.m. (2017-07-18 06:53:42 UTC) #1
ire
First patch ready :) https://codereview.adblockplus.org/29491561/diff/29491562/includes/styleguide/colors/backgrounds.md File includes/styleguide/colors/backgrounds.md (right): https://codereview.adblockplus.org/29491561/diff/29491562/includes/styleguide/colors/backgrounds.md#newcode11 includes/styleguide/colors/backgrounds.md:11: {: .bg-error } What do ...
July 18, 2017, 6:57 a.m. (2017-07-18 06:57:20 UTC) #2
juliandoucette
This is not quite what I had in mind. The Acceptable Ads website styleguide includes ...
July 24, 2017, 8:07 p.m. (2017-07-24 20:07:55 UTC) #3
ire
Thanks for the feedback :) As a broad note, which I'm not sure was clear, ...
July 25, 2017, 8:35 a.m. (2017-07-25 08:35:41 UTC) #4
juliandoucette
On 2017/07/25 08:35:41, ire wrote: > As a broad note, which I'm not sure was ...
July 25, 2017, 1:24 p.m. (2017-07-25 13:24:22 UTC) #5
juliandoucette
> I would have preferred if you started without any visual > separations and then ...
July 25, 2017, 1:33 p.m. (2017-07-25 13:33:33 UTC) #6
juliandoucette
Also, it seems like I've created two tasks out of this review. 1. Change website-defaults ...
July 25, 2017, 1:38 p.m. (2017-07-25 13:38:24 UTC) #7
ire
I think we need to decide *who* this styleguide is for first. I did not ...
July 26, 2017, 10:34 a.m. (2017-07-26 10:34:31 UTC) #8
ire
Here is a more stripped back version of the styleguide, using just website defaults CSS. ...
Aug. 3, 2017, 3:19 p.m. (2017-08-03 15:19:07 UTC) #9
juliandoucette
This patchset doesn't seem to apply to website-defaults.
Aug. 7, 2017, 2:12 p.m. (2017-08-07 14:12:22 UTC) #10
ire
Ready for review https://codereview.adblockplus.org/29491561/diff/29509658/includes/styleguide/style.html File includes/styleguide/style.html (right): https://codereview.adblockplus.org/29491561/diff/29509658/includes/styleguide/style.html#newcode1 includes/styleguide/style.html:1: <style> I added these basic styles ...
Aug. 8, 2017, 3:54 p.m. (2017-08-08 15:54:05 UTC) #11
ire
It occurred to me that perhaps we should split the sections into pages, like you ...
Aug. 9, 2017, 8:40 a.m. (2017-08-09 08:40:52 UTC) #12
juliandoucette
> It occurred to me that perhaps we should split the sections into pages, like ...
Aug. 9, 2017, 12:06 p.m. (2017-08-09 12:06:55 UTC) #13
ire
> I think a styleguide is a cheatsheet and a good cheatsheet fits on one ...
Aug. 9, 2017, 4:50 p.m. (2017-08-09 16:50:32 UTC) #14
juliandoucette
> I'm not sure why we need both a styleguide *and* the component demo pages, ...
Aug. 9, 2017, 7:56 p.m. (2017-08-09 19:56:18 UTC) #15
ire
On 2017/08/09 19:56:18, juliandoucette wrote: > > I'm not sure why we need both a ...
Aug. 10, 2017, 9:13 a.m. (2017-08-10 09:13:37 UTC) #16
ire
> I will upload a new patchset soon. Done.
Aug. 10, 2017, 9:53 a.m. (2017-08-10 09:53:30 UTC) #17
juliandoucette
Thanks Ire > Yes this makes sense. As a further suggestion, we could even have ...
Aug. 10, 2017, 4:49 p.m. (2017-08-10 16:49:32 UTC) #18
ire
Thanks. I've replied to the comment about different styleguides below. In this patch, I addressed ...
Aug. 11, 2017, 11:09 a.m. (2017-08-11 11:09:49 UTC) #19
juliandoucette
> Thanks. I've replied to the comment about different styleguides below. In this > patch, ...
Aug. 16, 2017, 2:42 p.m. (2017-08-16 14:42:02 UTC) #20
ire
On 2017/08/16 14:42:02, juliandoucette wrote: > > Thanks. I've replied to the comment about different ...
Aug. 16, 2017, 2:46 p.m. (2017-08-16 14:46:28 UTC) #21
juliandoucette
Looks good. https://codereview.adblockplus.org/29491561/diff/29511565/pages/content.html File pages/content.html (right): https://codereview.adblockplus.org/29491561/diff/29511565/pages/content.html#newcode16 pages/content.html:16: <h2 id="body-content">Body content</h2> On 2017/08/11 11:09:47, ire ...
Aug. 20, 2017, 4:48 p.m. (2017-08-20 16:48:39 UTC) #22
ire
Thanks. I've separated the the styleguide into a "Content Styleguide" and "Developer Styleguide".
Aug. 21, 2017, 10:19 a.m. (2017-08-21 10:19:52 UTC) #23
juliandoucette
Aug. 22, 2017, 2:37 p.m. (2017-08-22 14:37:54 UTC) #24
LGTM

Powered by Google App Engine
This is Rietveld