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

Adding classes to pagebreak.php and other improvements #14670

Closed
wants to merge 5 commits into from
Closed

Adding classes to pagebreak.php and other improvements #14670

wants to merge 5 commits into from

Conversation

AndySDH
Copy link
Contributor

@AndySDH AndySDH commented Mar 16, 2017

Since the navigation for multi-page articles (plugins/content/pagebreak.pagebreak.php) can't be template-overridden (unlike the article navigation in pagenavigation.php, which can), we could make some basic improvements to it in the core. This is what I did for my site and I think could be beneficial in general:

  • Add CSS classes to the prev/next links, so they can be stylized via CSS. Currently they have no class.

  • When we are on the first page, there is little point in showing a blank unclickable "prev" link. Same thing for when we are on the last page and the "next link", I think they shouldn't be shown at all.

  • Technically, it would be ideal to use (create) a different language string for these next/prev links, as they are not the generic next/prev article links, but they represent next page/previous page. By having a different language string (like JPREV_PAGE and JNEXT_PAGE) we can individually value these as "PREV PAGE" and "NEXT PAGE".

Summary of Changes

  • Added "prev_page" and "next_page" class
  • Added span around text
  • Removed previous link when on first page, and next link when on last page
  • Changed language string so that is different from the normal JPREV and JNEXT

Of course, if we do this, an additional language string with "PREV PAGE" and "NEXT PAGE" respectively should be created.

This is my first pull request and I had a hard time figuring out how to set it up so excuse any possible mistake.

Also remove the next/prev link if on last/first page respectively. No reason to keep an unclickable link, might as well not show it at all.
@ghost
Copy link

ghost commented Mar 16, 2017

@AndySDH do you have Test Instructions? Congrats to your first PR.

@AndySDH
Copy link
Contributor Author

AndySDH commented Mar 16, 2017

Thanks! I don't have any specific test instructions as there are just display changes or underlying code changes, as in, there is not much to "test" :D

@AndySDH
Copy link
Contributor Author

AndySDH commented Jul 26, 2017

Hello, what is the status of this small fix being added? Newer and newer Joomla updates keep coming out yet this keeps seeming to be left out, so I'm finding myself to always having to core-override it on each update.
Thanks :)

@brianteeman
Copy link
Contributor

Nothing has happened because no one has tested it.

@Quy
Copy link
Contributor

Quy commented Sep 14, 2017

I have tested this item 🔴 unsuccessfully on 320538e

Missing language strings.

On first page:
Notice: Undefined variable: prev in C:\xampp\htdocs\joomla-cms\plugins\content\pagebreak\pagebreak.php on line 378

On last page:
Notice: Undefined variable: next in C:\xampp\htdocs\joomla-cms\plugins\content\pagebreak\pagebreak.php on line 378


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14670.

@AndySDH
Copy link
Contributor Author

AndySDH commented Sep 14, 2017

Of course, if we do this, an additional language string with "PREV PAGE" and "NEXT PAGE" respectively should be created.

As I did mention, the language strings should of course be created, if someone with more wisdom with their locations and correct placements can add them to the correct language file for this PR, it would be appreciated, as I'm not familiar with the conventions you guys use there.

@Quy
Copy link
Contributor

Quy commented Jan 4, 2018

$prev and $next have to be initialized.

JNEXT and JPREV are located in \administrator\language\en-GB\en-GB.ini and \language\en-GB\en-GB.ini so the new strings should be added to these files.

@AndySDH
Copy link
Contributor Author

AndySDH commented Jan 5, 2018

Thank you.
I created a new Pull Request which is fully-working now that I gained more knowledge on how the PR work.
This is the new one: #19303, which is also updated to the current Joomla version.
I'm closing this since it's obsolete.

@AndySDH AndySDH closed this Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants