Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make link padding only apply to more specific cases #1600

Merged
merged 1 commit into from
Mar 5, 2017

Conversation

adamdruppe
Copy link
Contributor

The old rule was too general and cause silly looking spaces as Andrei
described here:

http://forum.dlang.org/post/ohjurtkzhvufqsahmimj@forum.dlang.org

This makes it only happen where I believe it was intended to happen:
on the hand-written index replacement, such as here:
https://dlang.org/phobos-prerelease/std_algorithm.html

where we have a long list of links all right next to each other under
the quickindex div.

@wilzbach
Copy link
Member

wilzbach commented Mar 5, 2017

The old rule was too general

(and was introduced three days ago):

#1593

CC @JackStouffer

Copy link
Member

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the quick fix!

@wilzbach
Copy link
Member

wilzbach commented Mar 5, 2017

though I didn't actually test it... is there an online preview for that?

Hmm while your CSS rule makes a lot of sense, I am not sure if it improves the situation for std.algorithm:

image

http://dtest.dlang.io/artifact/website-261e2794158ca0d40ba40d6e8230dd9447135a50-49809e8d61a51aa8f109fddf642ae1a7/web/phobos-prerelease/std_algorithm.html

@JackStouffer
Copy link
Member

@wilzbach Was just about to draw attention to that. Here's the before pic for reference

screen shot 2017-03-04 at 11 30 28 pm

The old rule was too general and cause silly looking spaces as Andrei
described here:

http://forum.dlang.org/post/ohjurtkzhvufqsahmimj@forum.dlang.org

This makes it only happen where I believe it was intended to happen:
on the hand-written index replacement, such as here:
https://dlang.org/phobos-prerelease/std_algorithm.html

where we have a long list of links all right next to each other under
the quickindex div.
@adamdruppe
Copy link
Contributor Author

adamdruppe commented Mar 5, 2017

Hmm, I didn't think about the line wrapping... so let's go back to padding-right... but I still want to do it only on a list. The quickindex isn't consistently present in the right places either :S

Well, let's try trusting the adjacent selector and special casing the second element, to act like the first has the space too (standard css afaik doesn't permit us to do a:has(+a) (and I know it isn't implemented in the browsers... my dom.d does allow it though :P dom.d rox)... let's try it.

@dlang-bot dlang-bot merged commit af0e0eb into dlang:master Mar 5, 2017
@PetarKirov
Copy link
Member

@adamdruppe there's a small CSS bug when there's room for only a single link in the rightmost column - in that case the next link goes on the next line, but gets padded to the right. Look at largestPartialIntersection:
screenshot_2017-03-05-10-49-07

screenshot_2017-03-05-10-51-58

@adamdruppe
Copy link
Contributor Author

Hmm, maybe margin instead of padding can help with that. But really what we might be forced to do is go through the source files and add a class name to target for these lists on the container.

But, I do think that's relatively minor so at least we're a step ahead from where we were last night.

BTW, I don't really wanna spend a lot of time on this since IMO these tables are just a hack papering over a ddoc deficiency. The other two generators can do this kind of thing automatically anyway...

@PetarKirov
Copy link
Member

But, I do think that's relatively minor so at least we're a step ahead from where we were last night.

Agreed. My comment just just FYI.

@wilzbach
Copy link
Member

wilzbach commented Mar 5, 2017

Meh it seems that if the links in std.functional would be proper links, it would also be affected:
dlang/phobos#5245

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

Successfully merging this pull request may close these issues.

5 participants