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

Allow priority to be set for Block and Inline Renderers #23

Merged
merged 3 commits into from
Dec 3, 2021

Conversation

SimonJulian
Copy link
Contributor

Hi,

I noticed when trying to add a custom renderer that there was no way to pass a priority option through (meaning that if a renderer for the element type had already been registered and returned a non-null result, the renderer added to the config wouldn't be used).

This PR allows a priority key to be set for block / inline renderers, but falls back to 0 if not found. (0 is the default priority, so without adding the new key nothing should change),

I have also updated the comments in the config file to have the correct array key (class instead of blockClass) and bought the installation setup documentation page to be inline with the config file (as many of the comments were already out of date before I made my own changes to the config file).

As an complete aside, locally I went to run the tests just to be sure nothing I changed caused any issues, but I was getting a number of failures even when I reverted my changes and I'm really not sure why.

@freekmurze
Copy link
Member

freekmurze commented Dec 2, 2021

Thanks for your work on this!

Meanwile, I fixed the tests on main. Could you merge in that branch so we see if the tests would also pass on this PR?

@SimonJulian
Copy link
Contributor Author

I've just done so, locally I'm still seeing 6 failures for some reason (which is a reduction from what I was seeing below).

I've attached the report.junit.xml that was generated as I wonder if something is odd my side but I'm not seeing anything oblivious.

report.junit.xml.txt

@freekmurze freekmurze merged commit 34cfb44 into spatie:main Dec 3, 2021
@freekmurze
Copy link
Member

All checks seem to have passed on GitHub 👍

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