Jump to content

Inconsistent results with $page->getMarkup


MoritzLost
 Share

Recommended Posts

A couple of times I've stumbled over a problem with $page->getMarkup / $page->getText. I'm often using this to parse module settings where the user can either enter a field name or a longer format with field replacements in curly braces. The problem is that entering a static text (not a field name) WITHOUT curly braces always returns an empty string. This seems counterintuitive, and it also presents a problem for those settings. More specific, for a module configuration where the user can enter a string that will be output somewhere. Something like this:

echo $page->getMarkup($modules->get('MyModule')->MyTextFormatSetting);

This works great for two out of three inputs:

  1. title -> Returns the page's title, great.
  2. The current title is {title} -> Returns the custom string with the page's title replaced in the end, great.
  3. My custom text without replacements -> Returns an empty string, not so great.

It's arguable whether getMarkup should just return the original text unmodified in the third case (imho it should). But how do I work around this when working on a module? I can think of two approaches:

  1. Add another setting where the user can switch between a static text and a replacement pattern for getMarkup. This is not ideal because it's another setting I have to code and the user has to think about, which should be unnecessary.
  2. Fallback to the original string if getMarkup returns an empty string. The problem here is if the original input is in fact a field name and the field is just empty on that field, I'm now outputting a field name instead of an empty string, which is incorrect in that case. I could work around that by checking if the original input is the name of an existing field, but that once again adds more complexity.

Is there a better way to do this? Some function I haven't considered? How do you handle this in your modules?

Any suggestions would be appreciated!

Link to comment
Share on other sites

I can see why it's awkward for your use case, but the getMarkup() and getText() methods are working as per their documentation as I understand it.

The argument can be one of two things: "field name" or "markup string with field {name} tags in it". The method has to distinguish between those two possibilities and it does that by looking for the presence of "{" and "}" - if those characters are not found it treats the string as a field name.

Basically you're requesting a new feature which is a third possible argument to these methods, namely a string that isn't a field name and doesn't have {name} tags in it. I'm sure Ryan would consider that if you raise it in the requests repo, but given how many existing requests there are you'll probably want to come up with a workaround in the meantime.

You could do this...

echo $page->getMarkup($modules->get('MyModule')->MyTextFormatSetting) ?: $modules->get('MyModule')->MyTextFormatSetting;
echo $page->getText($modules->get('MyModule')->MyTextFormatSetting) ?: $modules->get('MyModule')->MyTextFormatSetting;

...or in the case of getMarkup() the method is hookable so you could do this...

$wire->addHookAfter("Page::getMarkup", function(HookEvent $event) {
	$key = $event->arguments(0);
	if(!$event->return) $event->return = $key;
});

 

  • Like 3
Link to comment
Share on other sites

Thanks @Robin S, that was my conclusion as well.

Quote

I can see why it's awkward for your use case, but the getMarkup() and getText() methods are working as per their documentation as I understand it.

The argument can be one of two things: "field name" or "markup string with field {name} tags in it". The method has to distinguish between those two possibilities and it does that by looking for the presence of "{" and "}" - if those characters are not found it treats the string as a field name.

I would say that the behaviour in my third case is somewhat unspecified in the documentation – because the string is neither a field name nor a replacement pattern. I guess what I don't like about the implementation is that the distinction between the two cases is done by checking if the string contains curly braces. For my taste, it would be more explicit to check if the passed value is a field name and fall back to the curly brace implementation if it isn't. Though admittedly there are a lot of edge cases with subfields or alternate fields ... Maybe it would be best to add an optional parameter to those methods that forces one specific behaviour.

Quote

You could do this...


echo $page->getMarkup($modules->get('MyModule')->MyTextFormatSetting) ?: $modules->get('MyModule')->MyTextFormatSetting;
echo $page->getText($modules->get('MyModule')->MyTextFormatSetting) ?: $modules->get('MyModule')->MyTextFormatSetting;

I have considered that, but that will produce an 'incorrect' result (for my use case) in another case. Namely, if the passed value is a field name but the field happens to be empty on the current page, the field name will be output – even though an empty string would be the expected result in that case. So in order to get the expected result for every input, I'll have to:

  1. Return the output of $page->getMarkup if it is not empty.
  2. If it is empty, check if the original string is a fieldname (or sub-fieldname or similar).
    1. If it isn't, output the original string.
    2. If it is, output an empty string.

I would prefer if that logic was done inside getMarkup. I guess I'll open a feature request for that.

  • Like 1
Link to comment
Share on other sites

So I just opened an issue and closed it immediately because I realized that my proposed solution already exists in the form of wirePopulateStringTags ? It works perfectly for my use case, only requiring to use {title} with curly braces instead of title for the first example, which in a way is even more predictable.

Those functions should really have better visibility in the documentation ?

  • Like 3
  • Haha 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...