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

Add Bard data modifiers #6226

Merged
merged 21 commits into from Aug 15, 2022
Merged

Add Bard data modifiers #6226

merged 21 commits into from Aug 15, 2022

Conversation

jacksleight
Copy link
Contributor

@jacksleight jacksleight commented Jun 17, 2022

This PR adds three new modifiers that allow you to manipulate and render bard data in various ways:

  • bard_html: Converts any bard data to an HTML string (excluding sets)
  • bard_text: Converts any bard data to a plain text string (excluding sets)
  • bard_items: Converts any bard data to a flat array of ProseMirror nodes and marks

"bard data" can be either:

  • A raw value from a bard field (a ProseMirror document), with or without sets
  • An array of ProseMirror nodes
  • A single ProseMirror node

When combined with the existing string and array modifiers these can be useful in a number of ways, for example:

Extract a text snippet for a meta description (without the overhead of rendering and then removing the HTML tags)

{{ main_content | raw | bard_text | safe_truncate:90 }}

Get the read time of the content (when it also contains sets)

{{ main_content | raw | bard_text | read_time }}

Find and render the first image in the content

{{ main_content | raw | bard_items | where:type:image | first | bard_html }}

Output a list of links contained in the content

{{ links = main_content | raw | bard_items | where:type:link }}
{{ links }}
    {{ node | bard_text }} - {{ attrs:href }}
{{ /links }}

This would also be really useful when implementing footnotes in Bard. You could add a new inline node for the footnote anchors, and then use these modifiers to extract the list of footnotes and render them separately at the bottom of the page.

In antlers the raw modifier has to be in there otherwise the Bard modifiers receive the rendered HTML and not the array data. It'd be nice if that wasn't necessary, something I was talking about in this idea.

In blade raw is not necessary as these modifiers will also accept a Value object, so you can just pass that straight in.

Closes #4343

@jasonvarga
Copy link
Member

Let me know what you think.

If you were waiting for some approval before continuing, here it is! I have no problem with these sorts of modifiers being added. 👍

I haven't reviewed how they work though. I'll leave that until you're ready for it to be reviewed.

@jacksleight
Copy link
Contributor Author

jacksleight commented Jul 1, 2022

OK, great. I'll finish this off.

@jacksleight
Copy link
Contributor Author

jacksleight commented Jul 7, 2022

This is now ready for review. 👍

I was originally passing the result of bard_text through Stringy::collapseWhitespace because it produces cleaner output, but perhaps that should be left to the user to apply? I'm not really sure, for now I've removed it.

@jacksleight jacksleight marked this pull request as ready for review July 7, 2022 14:37
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

I think this is pretty cool but I initially expected bard_text to output everything with html. i.e. just strip out all the sets.

Could we maybe rename it to bard_flattened_text bard_plain_text or something better if you can think of it? Then we could eventually have bard_text give you the html.

@jasonvarga
Copy link
Member

I guess you can get the bard html by doing

{{ bard_field | where:type:text | pluck:text | join('') }}

It would be nice to do {{ bard_field | raw | bard_text }} though.

(And of course be able to remove the | raw at some point)

@jacksleight
Copy link
Contributor Author

Ah yeah, I see what you mean. I'll have a think and make some changes. 👍

@jacksleight jacksleight marked this pull request as draft July 14, 2022 16:12
@jacksleight
Copy link
Contributor Author

jacksleight commented Jul 21, 2022

The best alternative to bard_text I’ve come up with is bard_words. Which I think works? It accurately describes what’s returned and I think it makes it clear that HTML isn’t included.

I’ve also added a new bard_text modifier that does exactly what you expected the previous version to do, return the value without sets as rendered HTML. This is particularly useful if you have some raw bard data, like a node you’ve extracted using bard_items, that you want to render as HTML on its own. I’ve added an image example above that uses this.

Do you think bard_text should accept a parameter that lets you toggle antlers parsing in the returned HTML, just like the fieldtype does for the original value?

Or, would it be better if these modifiers received the Value objects and then returned new ones? That way the original bard configuration could be carried through, and raw wouldn’t be necessary either.

@jacksleight jacksleight marked this pull request as ready for review July 21, 2022 12:58
@jacksleight
Copy link
Contributor Author

jacksleight commented Aug 11, 2022

Could be outside the scope of this PR, but I’m using a “deep” version of the bard_items modifier locally, which will also traverse into sets (including replicator sets) and their values, finding all sets, nodes and marks all the way down. Handy if your top-level content field is some kind of page builder replicator, or you have words inside sets that you’d also like extracted.

https://gist.github.com/jacksleight/c54d95c0d5437fdd56218d85cc5e8e73

It is a bit more involved than the others and can only work from PHP/Blade at the moment.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

The best alternative to bard_text I’ve come up with is bard_words. Which I think works? It accurately describes what’s returned and I think it makes it clear that HTML isn’t included.

bard_words makes sense to me 👍

I’ve also added a new bard_text modifier that does exactly what you expected the previous version to do, return the value without sets as rendered HTML.

This is great too. But with the image example you provided, adding a bard_text modifier to output an <img> tag feels odd. Can we change it to bard_html?

Do you think bard_text should accept a parameter that lets you toggle antlers parsing in the returned HTML, just like the fieldtype does for the original value?

Ideally I think it should "just work" based on whether you've defined antlers on the field or not.

“deep” version of the bard_items modifier

Sounds handy but let's leave it out for now. Especially since it can't work in Antlers.

@jacksleight
Copy link
Contributor Author

Can we change it to bard_html?

No problem. Is there then an argument for renaming bard_words back to bard_text? bard_html gives you HTML, bard_text gives you plain text. Or am I overthinking this... probably. 😄

Ideally I think it should "just work" based on whether you've defined antlers on the field or not.

So at the moment that isn't possible because without access to the Value object and it's Fieldtype there's no way to know whether antlers was set on the original field.

@jasonvarga
Copy link
Member

jasonvarga commented Aug 11, 2022

So at the moment that isn't possible

Then I guess yeah we'll need an argument. I'd say it should not use Antlers by default and then you opt into it.

@jacksleight
Copy link
Contributor Author

jacksleight commented Aug 12, 2022

I've renamed bard_text to bard_html.

To add the antlers parsing I assume the best approach is copy what the Bard augmentor does in the textValue() method: create a new value object with the antlers option set, so it can be handled by the parser.

This works in the tests but unfortunately doesn't work in a template. It looks like the parser doesn't like modifier chains returning value objects.

This outputs the value unparsed:

{{ main_content | raw | bard_words:true }}

This errors with "Passed value cannot be an array":

{{ main_content | raw | bard_words:true | safe_truncate:90 }}

But this does work:

{{ test = main_content | raw | bard_words:true }}
{{ test }}

I'm not really sure how to resolve that.

Edit: Maybe a simpler and more general purpose solution would be a dedicated antlers modifier, a bit like how there's the markdown modifier?

@jasonvarga jasonvarga mentioned this pull request Aug 12, 2022
@jasonvarga
Copy link
Member

Antlers modifier is a good idea. I guess you can revert your changes and just assume that'll be there. #6489

@jacksleight
Copy link
Contributor Author

Awesome! Commit reverted, I think that's everything. 👍

@jasonvarga
Copy link
Member

Sorry, somehow I overlooked this part of your comment the other day:

Is there then an argument for renaming bard_words back to bard_text? bard_html gives you HTML, bard_text gives you plain text.

This PR will end up with:

  • bard_items giving you all the raw flattened ProseMirror stuff.
  • bard_html giving you the html for all the text bits. Sets are excluded. You can use it a specific prosemirror node like an image and still get html back, too.
  • bard_text giving you the plain text for all the text bits. Sets are excluded.

Makes sense to me!

@jacksleight
Copy link
Contributor Author

No worries, done!

@jacksleight
Copy link
Contributor Author

I might roll the functionality from the "deep" version into an addon. It'll work well alongside these modifiers but I'm thinking the use case might be a bit too specific for core.

@jasonvarga jasonvarga merged commit a74b670 into statamic:3.3 Aug 15, 2022
@jacksleight jacksleight deleted the bard-modifiers branch August 15, 2022 20:06
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.

read_time with Bard sets throws string conversion error
3 participants