Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(407)

Issue 29337873: Issue 3729 - Make "Block element" dialog adapt to variable window sizes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by Sebastian Noack
Modified:
4 years, 1 month ago
Reviewers:
saroyanm
CC:
Wladimir Palant, Thomas Greiner, kzar
Visibility:
Public.

Description

Issue 3729 - Make "Block element" dialog adapt to variable window sizes

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -5 lines) Patch
M block.html View 1 2 chunks +23 lines, -5 lines 0 comments Download

Messages

Total messages: 5
Sebastian Noack
4 years, 1 month ago (2016-03-03 14:14:00 UTC) #1
saroyanm
https://codereview.adblockplus.org/29337873/diff/29337874/block.html File block.html (right): https://codereview.adblockplus.org/29337873/diff/29337874/block.html#newcode42 block.html:42: font-family: Arial, Helvetica, sans-serif; Detail: Can you please reorder ...
4 years, 1 month ago (2016-03-03 15:29:25 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29337873/diff/29337874/block.html File block.html (right): https://codereview.adblockplus.org/29337873/diff/29337874/block.html#newcode42 block.html:42: font-family: Arial, Helvetica, sans-serif; On 2016/03/03 15:29:25, saroyanm wrote: ...
4 years, 1 month ago (2016-03-03 15:40:14 UTC) #3
saroyanm
LGTM https://codereview.adblockplus.org/29337873/diff/29337874/block.html File block.html (right): https://codereview.adblockplus.org/29337873/diff/29337874/block.html#newcode42 block.html:42: font-family: Arial, Helvetica, sans-serif; On 2016/03/03 15:40:14, Sebastian ...
4 years, 1 month ago (2016-03-03 16:11:34 UTC) #4
Sebastian Noack
4 years, 1 month ago (2016-03-03 17:42:33 UTC) #5
https://codereview.adblockplus.org/29337873/diff/29337874/block.html
File block.html (right):

https://codereview.adblockplus.org/29337873/diff/29337874/block.html#newcode42
block.html:42: font-family: Arial, Helvetica, sans-serif;
On 2016/03/03 16:11:34, saroyanm wrote:
> On 2016/03/03 15:40:14, Sebastian Noack wrote:
> > On 2016/03/03 15:29:25, saroyanm wrote:
> > > Detail: Can you please reorder css properties with the order specified in
> the
> > > https://adblockplus.org/en/coding-style 
> > 
> > Done, I think.
> 
> Yes only one small note: please move display properties on top.

I did leave the display property definition below as it is flex-box related,
which I think is also kinda inline with that WordPress article which states to
group similar properties.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5