Skip to content

Allow adding attributes without value - #986

Merged
colinodell merged 5 commits into
thephpleague:mainfrom
svenluijten:attributes-without-values
Jul 31, 2023
Merged

colinodell merged 5 commits into
thephpleague:mainfrom
svenluijten:attributes-without-values

Conversation

@svenluijten

Copy link
Copy Markdown
Contributor

As said in #985, this is my attempt at allowing valueless attributes to be set on elements.

@svenluijten

svenluijten commented Jul 29, 2023 •

Copy link
Copy Markdown
Contributor Author

Unfortunately the tests seem to fail as it is, probably because the regex is now too loose, causing a single functional test to fail; one that has to do with illegal whitespace in HTML. I'd appreciate some help to fix that if this is a feature you'd want included 🙂

@colinodell colinodell linked an issue Jul 31, 2023 that may be closed by this pull request
@colinodell

Copy link
Copy Markdown
Member

Thanks for working on this! I left some pointers on your original proposal which should hopefully help here.

Comment thread src/Util/RegexHelper.php Outdated
Comment thread src/Extension/Attributes/Util/AttributesHelper.php Outdated
Comment thread tests/unit/Extension/Attributes/Util/AttributesHelperTest.php Outdated
@svenluijten

Copy link
Copy Markdown
Contributor Author

Thanks for the pointers! I'll work them out later today. Meanwhile, do you want me to point this PR to main? Seeing as you added the issue to the 2.5 milestone?

@svenluijten
svenluijten requested a review from colinodell July 31, 2023 14:51
@colinodell

colinodell commented Jul 31, 2023 •

Copy link
Copy Markdown
Member

Meanwhile, do you want me to point this PR to main? Seeing as you added #985 to the 2.5 milestone?

Ah, yes please! Edit: looks like I can do that :)

@colinodell
colinodell changed the base branch from 2.4 to main July 31, 2023 15:55

@colinodell colinodell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@svenluijten

Copy link
Copy Markdown
Contributor Author

FWIW, this is what I was working on when I thought I could use valueless attributes 🙂

@colinodell
colinodell merged commit 2138460 into thephpleague:main Jul 31, 2023
@colinodell colinodell added this to the v2.5 milestone Jul 31, 2023
@colinodell

Copy link
Copy Markdown
Member

This change will be available in the future 2.5.0 release. It may be a little while before that gets tagged, as I'm hoping to include some additional (unrelated) changes at the same time.

@jasonvarga

jasonvarga commented Jul 22, 2024 •

Copy link
Copy Markdown
Contributor

Hello. This PR has unfortunately caused some issues for us.

Apologies for commenting on an old PR, but this has only just been tagged today.

I'm happy to try and open a PR to fix this myself, but I thought I'd mention it here first as you may understand the problem better.

When we have templating language within our markdown string, previously the markdown parser would leave it alone. Now, it basically clears it out. For example:

$converter->convert('# Hello {{ foo }}');

// Before: <h1>Hello {{ foo }}</h1>
// After: <h1>Hello {}</h1>

We use the AttributesExtension. If I revert the SINGLE_ATTRIBUTE const change from this PR, it works again (but obviously your feature wouldn't).

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.

Allow setting attributes without values in Attributes extension

3 participants