Pagination / Pager code review and better customizing

[tlindig]

Reviewed Pagination and Pager.

For Pagination:

• renamed variables to be conform with variables naming scheme (add '-default')
• add new variables for better customizing and states support (hover, active, disabled)
• rewrite rules for better customizing and states support (hover, active, disabled)
• use mixin ".pagination-size" also for base size
• removed z-index setting, because I did not see the need for

For Pager:

• removed variables @pager-disabled-color, because Pager use for all other color setting the variables of Pagination and the value of this variables was also the same that corresponding Pagination variables has
• add/rewrite rules for better customizing and states support (hover, active, disabled)

[mdo]

Sorry, but we cannot rename any variables until v4, unless you want to deprecate them instead. I'm not too happy about that though.

Also, your diff is a bit dirty—perhaps out of date with master?

[tlindig]

now done by the mixin ".pagination-size" at bottom of this file. I was not sure if it better to add this mixin here or in the "sizing" block.

[tlindig]

Sorry, but we cannot rename any variables until v4, unless you want to deprecate them instead. I'm not too happy about that though.

OK, I will do.

Also, your diff is a bit dirty—perhaps out of date with master?

But I can not explain why. my repo is up to date, I double checked it. May be some "changes" come in, because I work on a windows system. But I am not sure.

git.exe pull -v --progress       "upstream" master

From git://github.com/twbs/bootstrap
* branch            master     -> FETCH_HEAD
Already up-to-date.

Success (2667 ms @ 03.12.2013 10:50:35)

[tlindig]

Also, your diff is a bit dirty—perhaps out of date with master?

After I set the line feed to '\n' (see pull request #11687), the diff looks much better.

[mdo]

Diff looks better now, so make those updates given earlier feedback and we can revisit for v3.1.

[tlindig]

I'm not sure who you mean. Shall I make an update? If so, which update?

[mdo]

Here's more specific feedback for you to act on:

renamed variables to be conform with variables naming scheme (add '-default')

You cannot rename variables until a major version bump.

add new variables for better customizing and states support (hover, active, disabled)

Adding new variables is okay as long as they are backward compatible.

removed z-index setting, because I did not see the need for

We use the z-index to handle :hover and :active states because we use negative margins for spacing. Without it, the active and hovered items would be behind their next sibling.

removed variables @pager-disabled-color, because Pager use for all other color setting the variables of Pagination and the value of this variables was also the same that corresponding Pagination variables has

Again, you can't remove variables. You can deprecate for backward compatibility, but not straight up remove.

[tlindig]

You cannot rename variables until a major version bump.

undone this in commit 48236ba

Adding new variables is okay as long as they are backward compatible.

in commit 48236ba I initialized the old variables with the new one and add a hint, that they are deprecated.

We use the z-index to handle :hover and :active states because we use negative margins for spacing. Without it, the active and hovered items would be behind their next sibling.

OK, thank you for explanation. I now readd z-index (commit 18580af)

Again, you can't remove variables. You can deprecate for backward compatibility, but not straight up remove.

see commit 48236ba

Is it allowed to update the first post? May be this is the reason for misunderstanding or the not seen updates.

[cvrebert]

Could you squash the commits where you fixed the backward-compatibility issues?

[tlindig]

I will try, but I never did it before, so I am not sure, if I will be sucessfull.

I will do this tomorrow, it is late in Germany. Good night.

[tlindig]

I give it up. I do not find a way to reduce the count of commits. I tried git -rebase -i HEAD~9, and removed the unwanted commits, but if I want push the result, git says I have to make a pull before. If I do this, I got the unwanted commits merged again.

[tlindig]

made a new pull request.