michaelbromley / angularUtils

A place where I will collect useful re-usable Angular components that I make
MIT License
2k stars 861 forks source link

uiBreadcrumbs: Breadcrumbs are not evenly spaced #14

Closed sunild closed 10 years ago

sunild commented 10 years ago

In the demo as well as my own code I notice that the spacing between crumbs in the breadcrumb trail is not uniform.

screen shot 2014-06-25 at 7 40 06 pm

Notice that there is more space after the "/" than before it.

This seems to be caused by the template for the directive and (perhaps) the fact that Angular inserts comments into the DOM (comments around ng-repeat's, ng-if's, etc.).

The template for the breadcrumb trail inserts line breaks between the <li> and <a> tags that make up each breadcrumb element. It looks like this line break is part of the problem.

I can mitigate the issue somewhat by changing the template to remove those line breaks:

Modified template:

`

`

(NOTE: the above may not make the changes that I've made very noticeable. But the only line breaks in this template are after the opening <ol> tag AND before the closing </ol> tag.)

Also, note that I've inserted an &nbsp; after the displayName. That seems to even out the spacing. I think this &nbsp; is necessary to mitigate the effects of Angular adding comments to the DOM. In doing so, Angular seems to be inserting line breaks between the elements in the directive template.

I'm not sure how to best solve this. Note that if you look at the breadcrumb example on the Bootstrap site, that they do not insert line breaks in between the <li> and <a> tags. And if you do insert them that, you start to see the uneven spacing.

I think the template I pasted above is an OK solution, but I don't like that we have to stray from the Bootstrap example (by inserting the &nbsp;. Wondering if you have any thoughts about how to make the spacing between crumbs more uniform.

michaelbromley commented 10 years ago

Yeah I noticed this but never got round to investigating fully. Thanks for looking in to it.

It's a really bizarre issue. I can see that removing the line breaks does have a small effect, and adding the &nbsp; looks to me to sort it out, making it look the same as the example in the Bootstrap docs.

The strange thing is - why would we need to add a &nbsp; here when the Bootstrap example doesn't?

From this issue: https://github.com/twbs/bootstrap/pull/10467 , we can see that Bootstrap intentionally adds a space via the :before content in order to make extra space after the separator, but somehow there also seems to be space between the end of one <li> and the start of the next, as you can see in the image below, but I have no idea how or why this is there in the Bootstrap example, but not in this directive.

Native Bootstrap example

bootstrap_breadcrumbs_1

This directive

Yet in this directive, there is no gap, which is why we need to add the &nbsp; to make it look similar: bootstrap_breadcrumbs_2

I am reluctant to just change the template and add a purely presentational &nbsp; as a hack. After all, the directive is not specific to Bootstrap, and with other CSS frameworks (or no framework) this hack might have other undesirable consequences.

The real solution would be to figure out where that mysterious gap comes from, and then figure out how to replicate it without hacks (if possible).

sunild commented 10 years ago

I really do think the spacing might be messed up by Angular adding comments to the DOM. I tried deleting them in the web inspector, to no avail.

When I looked into trying to disable the comments, I found a post on Stack Overflow saying it wasn't possible. But notice the comment for this answer, which does seem to confirm that the comments might be the remaining culprit.

michaelbromley commented 10 years ago

Okay, after a bit more research, I am getting closer. Consider the actual HTML generated by Angular for the breadcrumbs directive (screenshot from Notepad++ so we can see the line breaks):

bootstrap_breadcrumbs_3

So there are 2 issues here. Firstly, as you correctly pointed out in your original issue, there should not be a line break between the opening <li> tag and the <a> tag. That gives an unwanted extra space.

The second issue is surprising. It turns out, the comments added by Angular in this case actually prevent the expected space from being inserted before the next <li>. For an explanation, see this article on CCS Tricks - in there Chris Coyier actually cites the use of comments as a way to prevent an unwanted space in an inline-block element. Turns out that Angular does this "trick" inadvertently when doing an ng-repeat on a list (or anything, for that matter).

This is why your other solution of manually adding in the &nbsp; works too. It simulates the space that would automatically be inserted if a line-break existed between the <li> elements.

Now that I understand what is going on, it seems acceptable to me to implement both of your suggestions, since the issues go beyond just Bootstrap, and would affect any CSS that defines the list items as inline-block elements (which, in the case of breadcrumbs, would probably be all of them).

Thanks again for the help.

sunild commented 10 years ago

This is great! Thanks for looking at it. A question and a comment for you:

I found that I was inserting the &nbsp; in the wrong place in the original HTML that I posted above. The &nbsp; should not be at the end of the anchor tag, it should be at the end of the list item tag. Otherwise the spacing is messed up for the $last list item (which has no anchor tag), and the links in the crumbs incorrectly have a space at the end:

<ol class="breadcrumb">
  <li ng-repeat="crumb in breadcrumbs" ng-class="{ active: $last }"><a ui-sref="{{ crumb.route }}" ng-if="!$last">{{ crumb.displayName }}</a><span ng-show="$last">{{ crumb.displayName }}</span>&nbsp;</li>
</ol>

Question: What tool did you use to see the HTML Angular is generating (the thing in your last screen shot)? It seems like a truer representation of the dynamically generated DOM than what I see when I use the browser's "web inspector".

michaelbromley commented 10 years ago

In my fix, I put the &nbsp; also before the closing tag, and it seems to be fine. The $last item has nothing following it so the spacing there is moot. In the Plunker demo it looks correct.

To get an accurate output of the HTML generated by Angular (or any other JS), inspect the element in Chrome dev tools, then right click the node you want to view, click "Copy as HTML" and then paste into a text editor.