Skip to content

Option to customize the TOC title#19

Merged
mtoensing merged 9 commits into
mtoensing:masterfrom
stracker-phil:master
Sep 19, 2022
Merged

Option to customize the TOC title#19
mtoensing merged 9 commits into
mtoensing:masterfrom
stracker-phil:master

Conversation

@divimode-philipp
Copy link
Copy Markdown
Contributor

This PR adds a new Text Control that allows authors to change the TOC title without the workaround of hiding the title and adding another title block.

Reason:
Using hide-title + creating a new block with the CSS class simpletoc-hidden is a complex task for our blog writers and therefore error-prone → they might set wrong header-level, forget to add the simpletoc-hidden class, or simply be too lazy and leave the default header.

Solution:
A new TextControl is displayed in the Block properties, where the block title can be overwritten. To stay consistent with existing pages, the default title is "Table of Contents", which is always used when an empty title is entered.

Preview:
Note how "Table of Contents" is inserted when the text field is cleared
2022-09-16_00-16-53 (1)

@mtoensing
Copy link
Copy Markdown
Owner

Thank you for this great idea and implementation. First thing I noticed: is there a reason why the default text does not use the localized string but a constant? That would be something for that would help many people who do not want to customize the title and start with the heading in their language. Just use the existing localized string.

I understand that you use the translated text later at the component but not as the default string. And maybe it is a good idea to use the fragment of the localized “table of contents” that is already translated in the help text.

Don’t get me wrong: I will merge this! But I need to understand if this is on purpose. :) thank you so much!

Copy link
Copy Markdown
Owner

@mtoensing mtoensing left a comment

Choose a reason for hiding this comment

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

I gave it a quick test in my German local installation and the string in the new input "Table of contents" does not get translated to the German string. Please verify that the existing localizations for the heading still work. =)

@divimode-philipp
Copy link
Copy Markdown
Contributor Author

Thanks for your thorough review, Marc!

I might be stuck at that part (I did not release a GB block before), so I'm not certain, how to update the translation file.
There is no translation for "Table of Contents" in the relevant .json file:

wp-content/languages/plugins/simpletoc-de_DE-dfbff627e6c248bcb3b61d7d06da9ca9.json
(screenshot below)

When I manually add ,"Table of Contents":["Inhaltsverzeichnis"] to that file, the translation instantly works.

I do not know how to update that file, but since it's inside the wp-contents/languages folder, I assume that the .json file automatically generated by wordpress.org once a new plugin version is committed. You might have more insights into that process... Also, the .json file contains the remark "generator: GlotPress..." which means, it was generated by the public WordPress repo, and not committed via GitHub/SVN

→ I believe that the translation will become available automatically once you bump the plugin version number and commit it to the wp.org repository.

no-table-of-contents-translation

@mtoensing
Copy link
Copy Markdown
Owner

mtoensing commented Sep 16, 2022

Hi,

The translations are being pulled online from the repository. There are no local translations in the plugins' code. There is an existing translation for "Table of Contents" that should be used:

__("Table of Contents", "simpletoc")

Just make sure to use exactly that with lower and uppercase. If you use it with the help text I would do it like this:

<TextControl
                label={__("Heading Text", "simpletoc")}
                help={__(
                  'Set the heading text of the block. The default value is:',
                  "simpletoc"
                ) + __("Table of Contents", "simpletoc") + '.'} 
                value={attributes.title_text}
                onChange={(value) =>
                  setAttributes({ title_text: value || __("Table of Contents", "simpletoc") })
                }
              />

That way it will at least show the correct one in the help text. But I think you should look the default definition of the attribute "title_text" in block.json

"attributes": {
		"no_title": {
			"type": "boolean",
			"default": false
		},
		"title_text": {
			"type": "text",
			"default": "Table of Contents"
		},


I don't know how translate that with the standard existing translation. And sadly this is very important because it is the headline of all existing installations. So maybe you don't define a default value there.

You can try:

"default": __("Table of Contents", "simpletoc")

in block.json but I don't think that will work...

@divimode-philipp
Copy link
Copy Markdown
Contributor Author

Translating the default value

It's not possible to translate strings in the block.json file. Use the second parameter of register_block_type() for this. - Info via WP Slack channel

This change adds the correct translation of the default value to the block:

// Old:
register_block_type( __DIR__ . '/build' , [
  'render_callback' => __NAMESPACE__ . '\\render_callback',
]);

// New:
register_block_type( __DIR__ . '/build' , [
  'render_callback' => __NAMESPACE__ . '\\render_callback',
  'attributes' => [
    'title_text' => [
      'default' => __( 'Table of Contents', 'simpletoc' ),
    ],
  ]
]);

Translating the string in edit.js

However, using {__("Table of Contents", "simpletoc")} in edit.js will become available after submitting the update to the WP plugin repository:

When you upload your plugin or theme to wordpress.org all JS files will automatically be parsed the same as is already being done for PHP files. Any detected translations will be added to translate.wordpress.org ...
...
Based on these parsed translations Language Packs are generated ...
...
- Found on https://make.wordpress.org/core/2018/11/09/new-javascript-i18n-support-in-wordpress/

i.e. the plugin repo will parse the JS file upon commit, detect the usage of the new __( 'Table of Contents', 'simpletoc' ) term and then add this term to the JSON language pack which is then used by the block editor

After committing the update to the plugin repo, two updates will be generated:

  1. The plugin update is distributed to all sites
  2. After the plugin updated, there will be a "New translations are available" notice → this will update the JSON pack for the block editor.

@mtoensing
Copy link
Copy Markdown
Owner

Yes but someone has to translate that string for the existing 13 languages. I can’t do that Strom those have to be approved - even if It is the same content. Therefore we need to make sure that the existing string is used. I can not approve a French translation. Even as the plug-in author. And people rely on that because the headline would be “table of contents” on many websites if they use another language. And I can’t approve strings. I know that sounds weird but please let us somehow use the important existing translations for the 13 languages for the headline string.

@mtoensing
Copy link
Copy Markdown
Owner

I am really sorry. The translation seems to work. But now the min_level is not saved anymore. It works in the master branch but in your current state of the pr/19 with no changes on my side I always get "no headings found"

Screen Shot 2022-09-16 at 23 17 57

Language does not matter. Do you see that on your end, too?

@mtoensing
Copy link
Copy Markdown
Owner

Screen Shot 2022-09-16 at 23 26 54

This seems to be the reason. You can't just add one attribute without block.json. You need to pass them all. I understand what you have tried but maybe there is another solution without using the default in the attribute definition? I

@mtoensing
Copy link
Copy Markdown
Owner

Do you need help? I really would like to see this option in the code! It is a great addition. But I hope you understand that we need to find a way to keep the existing translation for the string "Table of Contents" working for this. The reason is that everyone would lose their translated headline. You could argue:

"That is not a problem! they can change it or we wait until the translated strings on https://translate.wordpress.org/projects/wp-plugins/simpletoc/ appear"

But then everyone who has used SimpleTOC before and relied on the translated headline, will see the fallback to the englush version. So we have to make sure that the headline gets translated but can be overwritten by the user.

Philipp Stracker added 2 commits September 19, 2022 15:12
Undo part of b5a6405

Reason: When providing attributes via `register_block_type`, the attributes provided will overwrite the attributes from block.json (instead of extending those).

I.e. We need to provide ALL attributes via the register_block_type() function, or none.
Add 2 new filters to localize the block:

- Filter „load_script_translations“: Adds the translation for `{__(‚Table of Contents‘, ‚simpletoc‘ )}`
- Filter „block_type_metadata_settings“: Translates the default value of the TOC
@divimode-philipp
Copy link
Copy Markdown
Contributor Author

divimode-philipp commented Sep 19, 2022

Hi @mtoensing - I've updated the PR with a few changes:

  1. Revert the register_block_type() arguments. It turns out that this function should either get ALL "attributes", or none.
  2. Add filter load_script_translations: This filter injects the translation that's used by {__( "Table of Contents", "simpletoc" )} - provides compatibility/temporary solution, while the WP translation files are not updated yet.
  3. Add filter block_type_metadata_settings: This filter adds the translation of the block's default "title_text" attribute, without affecting the other attributes. We'll always need this filter

This worked in my tests on a German and English WP site. Do you see any problems with this logic?

Btw, thanks for your thorough tests and feedback above!

@mtoensing mtoensing merged commit f2d93a4 into mtoensing:master Sep 19, 2022
@mtoensing
Copy link
Copy Markdown
Owner

Thank you for not giving up. Great work. I will have a more profound look and wrap it up in a release as swiftly as I can.

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.

2 participants