It is likely that the most common use case of the module is just rendering the word permalink, formatted as a link, at the bottom of the content. So without the need for a copy box.

In that case, a large portion of the code can be skipped to improve the regeneration of the block in case the cache is cleared. We just have to insert a conditional that checks the setting No copy box and skips grabbing the site name, slogan, page title, and the whole generation of the HTML code snippet.

Furthermore, we might as well make that the default setting when enabling the module (no block title, no copy box).

CommentFileSizeAuthor
#9 permalink_block-copy_box_settings_install_fix-3133718-9.patch1.02 KBlolandese
#4 permalink_block-skip_unnecessary_code-3133718-4.patch15.23 KBlolandese
#3 permalink_block-skip_unnecessary_code-3133718-3.patch12.5 KBlolandese
#2 permalink_block-skip_unnecessary_code-3133718-2.patch12.83 KBlolandese

Comments

lolandese created an issue. See original summary.

lolandese’s picture

lolandese commented
Status: Active » Needs review
StatusFileSize
new permalink_block-skip_unnecessary_code-3133718-2.patch12.83 KB

The attached patch:

  • wraps all context-dependent code into the already existing if else statement, to only be executed if necessary
  • sets the default implementation to not show the block title and copy box
  • fixes grammar in the README
  • removes macOS files .DS_Store
  • adds a .gitignore file to avoid these annoyances in the future
  • fixes template and CSS files, splits them also to avoid the copy box JS when not needed.
lolandese’s picture

lolandese commented
StatusFileSize
new permalink_block-skip_unnecessary_code-3133718-3.patch12.5 KB

Let's try without the .gitignore. We try to delete the annoying macOS files, but these are also in the .gitignore.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	new file:   .gitignore
	modified:   README.txt
	deleted:    css/.DS_Store
	modified:   css/permalink_block.css
	new file:   css/permalink_block_copybox.css
	modified:   permalink_block.install
	modified:   permalink_block.libraries.yml
	deleted:    src/.DS_Store
	deleted:    src/Plugin/.DS_Store
	deleted:    src/Plugin/Block/.DS_Store
	modified:   src/Plugin/Block/PermaLinkBlock.php
	deleted:    templates/.DS_Store
	modified:   templates/permalink-block.html.twig
	deleted:    tests/.DS_Store
lolandese’s picture

lolandese commented
StatusFileSize
new permalink_block-skip_unnecessary_code-3133718-4.patch15.23 KB
lolandese’s picture

lolandese commented
Status: Needs review » Reviewed & tested by the community

Okay. Was solved using:
martin@martin-XPS-13-9370 /var/www/html/d9.localhost/web/modules/contrib/permalink_block (8.x-1.x +=) $ git diff-index 13968d58302ef3d6ea0121f4f20666b2fe5ea1d4 --binary > permalink_block-skip_unnecessary_code-3133718-4.patch

Source: git cannot apply binary patch *** without full index line - Stack Overflow

  • lolandese committed 9b27eba on 8.x-1.x 
    Issue #3133718 by lolandese: Improve performance: skip unnecessary code
lolandese’s picture

lolandese commented
Status: Reviewed & tested by the community » Fixed
lolandese’s picture

lolandese commented
Status: Fixed » Needs work

Module installation seems to have some hiccup. The Permalink shows correctly without the copy box, but No copybox is unselected in the block settings. Thus the copy box is rendered after saving the block settings "as is".

Furthermore, before we got a notice on module enabling, even with drush.

martin@martin-XPS-13-9370 /var/www/html/brown.localhost/web/modules/contrib/permalink_block (8.x-1.x *=) $ drush en permalink_block
 [success] Successfully enabled: permalink_block
 [notice] Message: All non-admin pages now contain a permalink. Change the settings at Structure 
 > Block Layout > Permalink [1].

[1] http://brown.localhost/admin/structure/block/manage/permalink_block

We lost that now:

martin@martin-XPS-13-9370 /var/www/html/d9.localhost/web/modules/contrib/permalink_block (8.x-1.x=) $ drush en permalink_block
 [success] Successfully enabled: permalink_block
lolandese’s picture

lolandese commented
Status: Needs work » Needs review
StatusFileSize
new permalink_block-copy_box_settings_install_fix-3133718-9.patch1.02 KB

The notice not visible in the terminal is a difference between Drush 10 and Drush 8. Probably it caused problems and they removed the feature. I tried to search the GitHub issue queue but could find anything clear on it. A pity, as I liked the feature.

Settings to suppress the copy box were in the wrong place, thus ignored. The patch fixes it.

  • lolandese committed 4fbbc40 on 8.x-1.x 
    Issue #3133718 by lolandese: Copy box settings install fix
lolandese’s picture

lolandese commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.