PCH Excerpt Suggestions: Add Persona and Tone settings#2776
Conversation
…o refactor/internal-api-stats # Conflicts: # tests/Integration/RestAPI/ContentHelper/ContentHelperFeatureTestTrait.php
…elper REST API Refactor: Base classes and Content Helper namespace implementation
|
Notes:
|
|
@vaurdan: regarding my previous comment, let's also keep in mind that there's a feature request to have Excerpt Suggestions in the PCH Sidebar. As this will probably affect how Excerpt Suggestions settings get saved, it would be good to factor this in during this work. If we're unsure of how this will play out, let's start implementing that request to avoid duplicate/refactoring work. |
|
@acicovic I went ahead and added the SettingsProvider to the component. It was a bit trickier, because of how we're using a SlotFill to insert the Excerpt panel to the Document sidebar (Using PluginDocumentSettingPanel). This required a bit of reworking on how the code is organized - instead of being a standalone module, I made it part of the From your original code, instead of having a new settings namespace for the excerpt generator, I changed it so it's also part of the main SidebarSettings namespace. Let me know your thoughts, and if you have any questions! |
|
Oh - I also renamed the component from |
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
Thank you for completing this work! I agree with renaming ExcerptGenerator to ExcerptSuggestions anywhere possible.
I've left a couple of comments, the only really important one being the one about the filter.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
…ent_helper_excerpt_generator`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@acicovic I believe I addressed all your feedback! Let me know if you have any additional thoughts 🙂 I believe since you're the owner of this PR, you can't approve it, but if we have 🟢 , let me know and I can approve it myself. |
acicovic
left a comment
acicovic
left a comment
There was a problem hiding this comment.
Thanks for addressing the filters situation!
I've left some minor comments.
|
I've left a comment in our last discussion, but otherwise I think we're good to go. Let's not merge this though, until our API refactoring branch is merged first. |
|
Closing in favor of #2890. |
3 participants
Description
This PR adds
PersonaandTonesettings to ourExcerpt Suggestionsfeature, in a similar manner to what already exists in ourTitle Suggestionsfeature.Motivation and context
How has this been tested?