<ol/> styles fixed #3

Merged
merged 1 commit into from Mar 27, 2016

3 participants

@axsy

Hello, sorry for my intermediate English. While <ul/> tag has correct default CSS styles <ol/> tag doesn't has it at all (for example see 'Portability' or 'Supporting Other Databases' sections in DBAL documentations).

@guilliamxavier guilliamxavier commented on the diff Mar 25, 2016
doctrine/static/layout.css
@@ -48,7 +48,7 @@ a:hover { color: #00508c; text-decoration:underline; border:0; }
#content h3 { margin:10px 0 20px 30px; font-weight:bold; font-size:130.0%; }
#content h4 { margin:10px 0 20px 30px; font-weight:bold; font-size:120.0%; }
#content h5 { margin:10px 0 20px 30px; font-weight:bold; font-size:110.0%; }
-#content ul li { margin:10px 0 0 30px; }
+#content ul li, #content ol li { margin:10px 0 0 30px; }
#content ul { margin-bottom: 10px; }

You should also insert “, #content ol” on line 52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilliamxavier

@beberlei @jwage @Ocramius @guilhermeblanco This is not just cosmetic, (I find that) it really has a negative impact on readability 😢 Please compare:

ol-1-a

(current http://docs.doctrine-project.org/en/latest/reference/working-with-objects.html#removing-entities) vs

ol-1-b

(after applying this PR).

Second example:

ol-2-a

(current http://docs.doctrine-project.org/en/latest/reference/working-with-associations.html#persistence-by-reachability-cascade-persist) vs

ol-2-b

(after applying the current PR as is) and

ol-2-b2

(after applying an augmented version of the current PR augmented as per the comment at https://github.com/doctrine/doctrine-sphinx-theme/pull/3/files#r57459336).

Aside from this last little omission (that you will be able to fix yourselves if needed, I imagine), is there anything preventing this 3-years-old PR from being merged?

@guilhermeblanco guilhermeblanco self-assigned this Mar 27, 2016
@guilhermeblanco guilhermeblanco merged commit d9bf324 into doctrine:master Mar 27, 2016
@guilliamxavier

Thank you @guilhermeblanco 😃
Note: the omission has been fixed with PR #7 (thanks to @mikeSimonson 😀)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment