Skip to content

[4.0] Custom fields prefix and suffix#24232

Merged
wilsonge merged 8 commits intojoomla:4.0-devfrom
brianteeman:prefix
Apr 23, 2019
Merged

[4.0] Custom fields prefix and suffix#24232
wilsonge merged 8 commits intojoomla:4.0-devfrom
brianteeman:prefix

Conversation

@brianteeman
Copy link
Contributor

J4 version of #23499

It is very common to need to display a unit of measurement/value with a field. Currently there is no way to do this without creating custom layouts for every field and every unit when needed.

This PR adds two new param to every custom field "Suffix" and "Prefix" which allows you to add a unit of measurement (or any other text) to be rendered after/before the value of the field.

The new field is disabled by default so there is no b/c issue

J4 version of joomla#23499

It is very common to need to display a unit of measurement/value with a field. Currently there is no way to do this without creating custom layouts for every field and every unit when needed.

This PR adds two new param to every custom field "Suffix" and "Prefix" which allows you to add a unit of measurement (or any other text) to be rendered after/before the value of the field.

The new field is disabled by default so three is no b/c issue

### To test
Apply the pr and then create a new custom field and you will see the new Prefix Suffix param in the render options.
@AndySDH
Copy link
Contributor

AndySDH commented Mar 18, 2019

Nice :)

Can you make it so that there are no spaces between the field value and the prefix/suffix?
So that you can do things like "200€" or "$500". And if you need spaces like " minutes" you would add the space while filling the input field itself. What do you think?

Also, as mentioned in the other issue, is having the "Show suffix" and "Show prefix" yes/no toggles necessary? Wouldn't be simpler to just have two text fields that would display only if not empty?

@brianteeman
Copy link
Contributor Author

Can you make it so that there are no spaces between the field value and the prefix/suffix?

There aren't any

@AndySDH
Copy link
Contributor

AndySDH commented Mar 19, 2019

There are for me:

image

@Bakual
Copy link
Contributor

Bakual commented Mar 19, 2019

There aren't any

I guess the spaces come from either the tab or linebreak in the PHP code (depending if suffix or prefix).

<?php if ($showPrefix == 1) : ?>
	<span class="field-prefix"><?php echo htmlentities($prefix, ENT_QUOTES | ENT_IGNORE, 'UTF-8'); ?></span>
<?php endif; ?>

There is whitespace between the ?> and the <?php which can cause this.

Also, as mentioned in the other issue, is having the "Show suffix" and "Show prefix" yes/no toggles necessary? Wouldn't be simpler to just have two text fields that would display only if not empty?

I'd second this. Just the remove the yes/no toggle and show the prefix/suffix if something is entered.
So instead of <?php if ($showPrefix == 1) : ?> do <?php if ($prefix) : ?>. That would make both the code and the UI simpler.
Or do you see a usecase for the toggle?

@ghost ghost added the J4 Issue label Apr 5, 2019
@ghost ghost removed the J4 Issue label Apr 13, 2019
# Conflicts:
#	components/com_fields/layouts/field/render.php
@brianteeman
Copy link
Contributor Author

Updated to remove conflicts and simplify as per @Bakual suggestion

<?php if ($showLabel == 1) : ?>
<span class="field-label <?php echo $labelClass; ?>"><?php echo htmlentities($label, ENT_QUOTES | ENT_IGNORE, 'UTF-8'); ?>: </span>
<?php endif; ?>
<?php if ($prefix) : ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just from a code review perspective. I think this needs to be <?php if (!empty($prefix)) : ?> otherwise you're going to get the span with an empty string. But if you test and tell me this works then I'm happy with that too

Copy link
Member

Choose a reason for hiding this comment

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

would work without empty but would raise a warning if $prefix is not defined.

Copy link
Member

Choose a reason for hiding this comment

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

But would not work if $prefix is "0" (zero)

Copy link
Contributor

Choose a reason for hiding this comment

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

The field params is a registry object, so it will default to an empty string if existing fields haven't been saved yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested and you do not get a span with empty string
Also tested what happens if you have a prefix and then delete the prefix - no span then either

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a point in what Harald wrote. It doesn't work if you have a prefix (eg "0") which evaluates to false. I'm not sure if you ever get such a prefix or suffix. If you want to avoid that, one could do a check if ($prefix !== '')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if prefix is 0 then you dont get a prefix (including the spans) not sure that would ever happen

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what I mean. I don't think "0" is a valid prefix or suffix. But I learned people do expect crazy things and maybe someone expects "0" to appear.
Personally, I don't mind 😄

@HLeithner
Copy link
Member

I see another problem with this approach:

$prefix = Text::plural($field->params->get('prefix'), $value);
$suffix = Text::plural($field->params->get('suffix'), $value);

if the prefix language string for plural needs a suffix like 1st 2nd... this maybe and with a space between prefix and suffix.

@brianteeman
Copy link
Contributor Author

Sorry I dont understand your plural comment

@HLeithner
Copy link
Member

HLeithner commented Apr 23, 2019

I'm not sure if a "decoration" attribute with a place holder wouldn't be better

something like:

DECORATION_1="The %dst floor"
DECORATION_2="The %dnd floor"
DECORATION_3="The %drd floor"
DECORATION_N="The %dth floor"

$decoration = Text::plural($field->params->get('decoration'), $value);

And replaces the echo $value completely

@wilsonge
Copy link
Contributor

How is that not covered by passing this through the Text::Plural already?

@Bakual
Copy link
Contributor

Bakual commented Apr 23, 2019

1st, 2nd, 3rd is impossible to do with Text::plural. Because it's not plural forms were talking here. Plurals forms are defined per language and eg in english you only have two forms. It's either plural or singular. So afaik you can't do a "3rd".

@HLeithner
Copy link
Member

1st, 2nd, 3rd is impossible to do with Text::plural. Because it's not plural forms were talking here. Plurals forms are defined per language and eg in english you only have two forms. It's either plural or singular. So afaik you can't do a "3rd".

what a mess...

@wilsonge wilsonge merged commit 2cfa81c into joomla:4.0-dev Apr 23, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 23, 2019
@brianteeman
Copy link
Contributor Author

Thanks

@brianteeman brianteeman deleted the prefix branch April 23, 2019 10:36
@wilsonge
Copy link
Contributor

I think this is good enough for now. As Harald has pointed out it doesn't cover every case. But we can work on that if there's demand. I think it's useful enough as it is now.

Thanks!

fancyFranci pushed a commit to fancyFranci/joomla-cms that referenced this pull request Apr 24, 2019
J4 version of joomla#23499

It is very common to need to display a unit of measurement/value with a field. Currently there is no way to do this without creating custom layouts for every field and every unit when needed.

This PR adds two new param to every custom field "Suffix" and "Prefix" which allows you to add a unit of measurement (or any other text) to be rendered after/before the value of the field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants