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

[Ticket/14203]Modularize and fix paging styling #3938

Closed
wants to merge 3 commits into from
Closed

[Ticket/14203]Modularize and fix paging styling #3938

wants to merge 3 commits into from

Conversation

hanakin
Copy link
Member

@hanakin hanakin commented Sep 27, 2015

@bantu
Copy link
Collaborator

bantu commented Sep 28, 2015

@hanakin I am sorry to inform you that we still need the PHPBB3-12345 id in each commit message as per https://wiki.phpbb.com/Git#Commit_Messages. You (or one of us) will have to adjust all your commits.

@hanakin
Copy link
Member Author

hanakin commented Sep 28, 2015

@bantu can you clear up what this is this even used for as it seems sort of superfluous as we already have the ticket id in the message? I will try and refactor all my open PRs, but it just seems this could easily be factored out as a requirement if its a hook using some sort of global for the "PHPBB3" and the "12345" from [ticket/12345].

@bantu
Copy link
Collaborator

bantu commented Sep 28, 2015

@hanakin The thing that matters for pinning commits to tracker tickets is the PHPBB3-12345 part. The message part is less important. Using "[feature/somefeaturename] Bla bla foo bar." is fine for the top of the message as well. There used to be a check on Travis CI ensuring all commits are well formatted. I am not sure whether it has been disabled or whether it is buggy.

.pagination li a:hover, .pagination li a:hover .icon, .pagination .dropdown-visible a.dropdown-trigger, .nojs .pagination .dropdown-container:hover a.dropdown-trigger {
.button-paging:hover,
.button-paging:focus,
.button-active {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the last selectors whose purpose is to keep the page jump button active while the dropdown is open.

Copy link
Member Author

Choose a reason for hiding this comment

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

needs addressed but not all of it is necessary ill get to it tommorow

@prototech
Copy link
Contributor

The next/previous/page jump icons are not vertically aligned to the center. They seem about 2px too high.

@hanakin
Copy link
Member Author

hanakin commented Oct 17, 2015

what browser/OS is it appearing offset i do not see it offset!

@Nicofuma Nicofuma modified the milestones: 3.2.0-a1, 3.2.0-a2 Oct 17, 2015
@prototech
Copy link
Contributor

Ubuntu/Chromium
pagination

@hanakin
Copy link
Member Author

hanakin commented Nov 11, 2015

ready for review @VSEphpbb @cyberalien @prototech

@hanakin
Copy link
Member Author

hanakin commented Nov 16, 2015

@prototech similar to #3937 this is to simplify the use of buttons and standardize the code base. It also makes working with icons easier

@hanakin
Copy link
Member Author

hanakin commented Nov 17, 2015

@prototech ill let you decide on this one it has a lot of merit for making the paging easier to work with and a modular class to use elsewhere but per our discussion there are some BC issues

@Nicofuma
Copy link
Member

Nicofuma commented Dec 7, 2015

@prototech what about the BC issue? The alpha2 is coming soon and after hat it will be too late (or almost) to introduce any new BC break.

@Nicofuma Nicofuma modified the milestones: 3.2.0-a2, 3.2.0-a3 Dec 19, 2015
@Nicofuma Nicofuma removed this from the 3.2.0-b1 milestone Feb 4, 2016
@Nicofuma Nicofuma modified the milestones: 3.2.0-b2, 3.2.0-b1, 3.3.0-a1 Feb 4, 2016
@hanakin hanakin closed this Mar 7, 2017
@hanakin hanakin deleted the ticket/14203 branch May 17, 2017 14:57
@hanakin hanakin removed this from the 4.0.0-a1 milestone Mar 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants