Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Update template/pagination/pagination.html #174

Closed
wants to merge 1 commit into from
Closed

Update template/pagination/pagination.html #174

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2013

Display the pagination links with the right cursor.

Display the pagination links with the right cursor.
@petebacondarwin
Copy link
Member

Should this not be done with CSS?

@pkozlowski-opensource
Copy link
Member

@christianotter what do you mean exactly by "the right cursor"? I had a brief look and can't see CSS selector it twitter's bootstrap that would match based on its href attrib. I don't mind merging your PR as it is not harmful but would like to make sure that we are fixing the right problem.

@petebacondarwin
Copy link
Member

Actually when you say it does no harm, you ought to consider this line:
https://github.com/angular/angular.js/blob/master/src/ng/directive/a.js#L38

On 28 February 2013 11:26, Pawel Kozlowski [email protected] wrote:

@christianotter https://github.com/christianotter what do you mean
exactly by "the right cursor"? I had a brief look and can't see CSS
selector it twitter's bootstrap that would match based on its href
attrib. I don't mind merging your PR as it is not harmful but would like to
make sure that we are fixing the right problem.


Reply to this email directly or view it on GitHubhttps://github.com//pull/174#issuecomment-14228854
.

@pkozlowski-opensource
Copy link
Member

Yes, I know this code, but it is testing for href value, not the attribute presence. BTW: we've got similar empty href in tabs: https://github.com/angular-ui/bootstrap/blob/master/template/tabs/tabs.html#L4

So, unless I'm mistaken it really doesn't do any harm as the current AngularJS implementation stands. But yes, I would like to better understand what is going on with this cursor.

@petebacondarwin
Copy link
Member

I think this is relevant: http://stackoverflow.com/a/2409858/287070.
Currently bootstrap has nothing to say about the cursor CSS property of
tags but most browsers seem to default to cursor: auto, which obviously
works out that if you don't put a href then it is not clickable!

Personally, I would prefer that this is done in CSS.

.pagination a {
cursor: link;
}

On 28 February 2013 11:54, Pawel Kozlowski [email protected] wrote:

Yes, I know this code, but it is testing for href value, not the attribute
presence. BTW: we've got similar empty href in tabs:
https://github.com/angular-ui/bootstrap/blob/master/template/tabs/tabs.html#L4

So, unless I'm mistaken it really doesn't do any harm as the current
AngularJS implementation stands. But yes, I would like to better understand
what is going on with this cursor.


Reply to this email directly or view it on GitHubhttps://github.com//pull/174#issuecomment-14229839
.

@pkozlowski-opensource
Copy link
Member

@christianotter Finally I think that I agree with @petebacondarwin. We shouldn't be adding "dummy" attributes for the CSS purposes if this can be avoided. Going to close this PR for now and remove href from other templates.

Please do ping and convince us if you feel strongly otherwise.

@bajamircea
Copy link

@pkozlowski-opensource the problem is that cursor is broken in angular-ui bootstrap pagination (Firefox Ubuntu/Windows and IE) compared with the one in bootstrap.

Not having a fix because it's not the best, still leaves the issue.

Options to fix, so that it works for users out the box, in order of ease are:

  1. add the href?
  2. somehow add the css automatically
  3. convince bootstrap to add a style for a issue that does not impact them?
  4. change browsers to show cursor if href is missing? LOL

Option 1 seems cheapest.

@pkozlowski-opensource
Copy link
Member

@bajamircea I understand this but we've got a bit of a problem here as those additional href attributes were breaking apps of several people - basically those links were triggering navigation (page reload). Sadly I never saw a reproduce scenario for this reload case so hard to say what was the exact problem.

So option (1) is the "cheapest" but apparently breaks some applications. I would try (3) but it might be hard in practice.

In any case dropping href attributes is the AngularJS-way of creating clickable links. So we seem to have a bit of a conflict here between Bootstrap's spirit and AngularJS spirit.

Not sure what is the best curse of actions here... If the reloading doesn't affect your web application you can always change default templates and add those missing attributes.

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

Successfully merging this pull request may close these issues.

4 participants