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

Custom Fields - display units #23499

Closed
wants to merge 2 commits into from

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jan 10, 2019

It is very common to need to display a unit of measurement 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 a new param to every custom field "Suffix" which allows you to add a unit of measurement (or any other text) to be rendered after 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 Suffix param in the render options.
image

Example usage

Before

image

After

image

##Plurals
If you need to have a different suffix for a plural then you can do this as follows

  1. In the suffix field enter a key such as MINUTE_N
  2. Create a language override MINUTE_N = Minutes
  3. Create any additional language overrides for specific numbers eg MINUTE_N_1 = Minute

The output will now be
0 Minutes
1 Minute
10 Minutes

It is very common to need to display a unit of measurement 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 a new param to every custom field "Suffix" which allows you to add a unit of measurement (or any other text) to be rendered after 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 Suffix param in the render options.

## Example usage
### Before

### After
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Jan 10, 2019
@wilsonge
Copy link
Contributor

This is nice but i guess needs to be translatable so it can be run through Text::plural so that you can have mile vs miles, minute vs minutes etc etc

@brianteeman
Copy link
Contributor Author

How is that possible?

@Bakual
Copy link
Contributor

Bakual commented Jan 10, 2019

For the rendering part, it would be possible, but the user would need to understand how plural strings work. You would have to enter a string like CUSTOM_UNIT. In the language override manager you then add the strings CUSTOM_UNIT_0, CUSTOM_UNIT_1 and CUSTOM_UNIT (for en-GB, other languages need different suffixes and maybe more or less strings) with the respective values.
When rendered, you pass the value and string through Text::plural. Eg Text::plural('CUSTOM_UNIT', 5);

edit: corrected my example which was wrong

@brianteeman
Copy link
Contributor Author

It would be pretty pointless to do that and overly complex.

@Bakual
Copy link
Contributor

Bakual commented Jan 10, 2019

For english and german it's certainly not something that is needed often. We usually just write the plural.
I honestly don't know how such things are handled in other languages.

@brianteeman
Copy link
Contributor Author

There are some things that can never be "easily" automated or translated. If someone really needs it then they can create an override but I doubt they ever will..

@ot2sen
Copy link
Contributor

ot2sen commented Jan 10, 2019

For the recipe example, it would be useful, in Danish at least.
Servings: 1 Adult
Preparation time: 1 Hour

Nice improvement btw. :)

@brianteeman
Copy link
Contributor Author

Good luck writing the code to do that so it is usable - but I guess I will just have to make the changes in my own sites manually

@Bakual
Copy link
Contributor

Bakual commented Jan 10, 2019

I don't know. The code line is easy. Just pass the unit string through that Text::plural() function together with the value of the field. Or do I miss something?
If the user doesn't need that feature, the string will be rendered unchanged. If the user adds all the necessary language strings manually (eg in the language override manager) and knows what he does, it will be nicely translated. Same thing we do in other places where user input may be translatable.

@AndySDH
Copy link
Contributor

AndySDH commented Jan 11, 2019

This is a very nice idea. It would save the trouble of using layout overrides for the suffix.

It's also useful to have a "Prefix" option as well. Think of prices. You probably want to prepend a dollar icon, like: $200.

What would also be cool (another thing that I've been doing with custom layouts so far), is the ability to set a number_format for numeric values (so for text fields and for integer fields), allowing to set separators for thousands and decimals.

So for example, 365100 can be displayed as 365,100.
Or, 25.368274 (decimals) can be limited to 2 decimals as: 25.37.

Just throwing the idea out there as well. All stuff that I have been doing with layout overrides so far.

@brianteeman
Copy link
Contributor Author

@AndySDH that is an interesting idea but off topic for this PR as it relates to just the integer field - this PR is for all fields.

@brianteeman
Copy link
Contributor Author

Same thing we do in other places where user input may be translatable.

Can you point to an example please

@Bakual
Copy link
Contributor

Bakual commented Jan 11, 2019

Can you point to an example please

The title of a custom field is translatable, just a few lines above your added code.

$label = JText::_($field->label);

It's not using the plural method in that case, but the concept should work same.

@brianteeman
Copy link
Contributor Author

Updated code (and instructions above) for plurals

@Bakual
Copy link
Contributor

Bakual commented Jan 11, 2019

I have tested this item ✅ successfully on 0973b87

Test units, including plural. Worked fine.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23499.

@AndySDH
Copy link
Contributor

AndySDH commented Jan 11, 2019

Issue I just realized with the code. This breaks out of the "field-value" span, as it sits outside of it. Therefore breaking the style of the value part.

The suffix span should instead be part of the field-value span as well.

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

@brianteeman
Copy link
Contributor Author

Why? This way allows styling for the suffix independent of the value

@brianteeman
Copy link
Contributor Author

a new field cannot break your layout!

@AndySDH
Copy link
Contributor

AndySDH commented Jan 11, 2019

Here is what I mean happens now:

image

image

But up to you. I can override the render.php and fix it for my sites. So that's no problem. But I might not be the only one that took this styling approach.

@brianteeman
Copy link
Contributor Author

So you would just adjust your template as you would whenever you add something new.

@Bakual
Copy link
Contributor

Bakual commented Jan 11, 2019

Either overriding the JLayout or adjusting the CSS allows to fix this for you.
It always depends on your usecase, what structure would be better. For example if you have mixed fields, some with and some without units (or with different units) you can align the numbers better with Brians approach.

@AndySDH
Copy link
Contributor

AndySDH commented Jan 11, 2019

I have tested this item ✅ successfully on 0973b87


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23499.

@Quy
Copy link
Contributor

Quy commented Jan 11, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23499.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 11, 2019
@AndySDH
Copy link
Contributor

AndySDH commented Jan 11, 2019

Just throwing out a couple of thoughts. Feel free to bash them.

  • Do we need the "Show Suffix" option? I think we could simply have the "Suffix" field, and in the layout file simply check if it's empty or not. If it's empty, don't display it. If not empty, display it. No need for an extra toggle imho.

  • And I think we could easily add a "Prefix" along with it, with the same exact logic as the suffix. So it would become something simple like:

image

Just my two cents anyway.

@Bakual
Copy link
Contributor

Bakual commented Jan 12, 2019

Only what is written in the Meeting notes (https://volunteers.joomla.org/departments/production/reports/936-production-meeting-december-2018):

It was voted to reject this and stick to the current plan of having 3.10 as the bridge release for Joomla 4

I don't really have more informations. @wilsonge or any other PD member could explain it better.

@brianteeman
Copy link
Contributor Author

ah - that is what in the missing sentence

chrome_2019-01-12_18-20-41

@Bakual
Copy link
Contributor

Bakual commented Jan 12, 2019

Yeah, something is missing there 😄

@AndySDH
Copy link
Contributor

AndySDH commented Jan 12, 2019

We could release minors every three months or whatever. It just needs someone willing to do the release work. That's the limiting ressource.

Eh, I see. Wish I had the skills to do it!

so that they can inform people when they submit a PR that it wont see the light of day until 2020 - so much for release often

I know right. So disheartening... :(

@mbabker
Copy link
Contributor

mbabker commented Jan 12, 2019

We could release minors every three months or whatever. It just needs someone willing to do the release work. That's the limiting ressource.

Eh, I see. Wish I had the skills to do it!

For the record, a minor release involves much more than just running the packaging script and publishing the release. Feature releases typically involve a larger number of project teams and resources, so a quick release cadence puts more work (and more burn out) on a higher number of project teams. Likewise, a quick release cadence alienates end users (even after having almost monthly releases for two years end users still groan that is too often). For a project the scale of Joomla, it is not feasible to push minor releases every time a feature is ready to ship to production.

@AndySDH
Copy link
Contributor

AndySDH commented Jan 12, 2019

For the record, a minor release involves much more than just running the packaging script and publishing the release. Feature releases typically involve a larger number of project teams and resources, so a quick release cadence puts more work (and more burn out) on a higher number of project teams. Likewise, a quick release cadence alienates end users (even after having almost monthly releases for two years end users still groan that is too often). For a project the scale of Joomla, it is not feasible to push minor releases every time a feature is ready to ship to production.

Well yeah, of course. I'm by no means saying there should be one every time there is a new feature ready, of course. But not even one release per year. There has to be a happy medium. Especially in this case where it's established for sure that it would take a full year for the next release.

@mbabker
Copy link
Contributor

mbabker commented Jan 12, 2019

One or two feature releases per year is that happy medium if you are one of the ones who are actively involved in the release processes (actual release publishing, marketing, documentation, translations, etc.). Anything more than that is asking for trouble, unless you are the CEO of a corporation using your company to force things to happen in your open source software to fulfill your for-profit organization's needs and investor promises.

@AndySDH
Copy link
Contributor

AndySDH commented Jan 12, 2019

Two would be an upgrade already. Especially now that with 3.10 being coupled with 4.0, it might get delayed etc. so there could potentially be 1.5+ year with no features getting merged... just because it has been decided that the number 3.10 "has" to be the last. No need to self-impose that restriction imho.
Oh well. Let's go back watching NXT UK Takeover.

@mbabker
Copy link
Contributor

mbabker commented Jan 12, 2019

3.9 was originally supposed to be the 4.0 bridge. Plans change.

Here's the issue from a pure code perspective. 4.0 is still 5 releases behind staging, this includes the entirety of 3.9. As long as features continue landing in 3.x, that just adds to the backlog of things that have to be merged forward to 4.0 (and then adapted to the 4.0 code). So moving forward on 3.x just makes it harder to move forward on 4.0 until 4.0 is caught up with 3.x (and it should never fall behind again once caught up). Sooner or later, you have to prioritize shipping 4.0 over continuing to ship 3.x feature releases, otherwise you continue to have the moving target of 4.0 with no realistic milestone in sight and continue splitting the few resources there are actually working on getting 4.0 shipped (outside of this elusive template team which seems to be the only 4.0 only group of people, most people working on 4.0 stuff are also working on 3.x maintenance). So sooner or later, it actually is a good thing that someone comes up and says "OK, 3.x feature releases are slowing down so we can actually ship 4.0 while PHP is still a relevant programming language".

@AndySDH
Copy link
Contributor

AndySDH commented Jan 12, 2019

3.9 was originally supposed to be the 4.0 bridge. Plans change.

Here's the issue from a pure code perspective. 4.0 is still 5 releases behind staging, this includes the entirety of 3.9. As long as features continue landing in 3.x, that just adds to the backlog of things that have to be merged forward to 4.0 (and then adapted to the 4.0 code). So moving forward on 3.x just makes it harder to move forward on 4.0 until 4.0 is caught up with 3.x (and it should never fall behind again once caught up). Sooner or later, you have to prioritize shipping 4.0 over continuing to ship 3.x feature releases, otherwise you continue to have the moving target of 4.0 with no realistic milestone in sight and continue splitting the few resources there are actually working on getting 4.0 shipped (outside of this elusive template team which seems to be the only 4.0 only group of people, most people working on 4.0 stuff are also working on 3.x maintenance). So sooner or later, it actually is a good thing that someone comes up and says "OK, 3.x feature releases are slowing down so we can actually ship 4.0 while PHP is still a relevant programming language".

LOL. It makes sense when put like that. Although I imagine that both bugfixes or features is stuff that needs to be merged forward anyway, so I assume it doesn't make much of a difference whether it's bugfix or a feature, especially if small like in this example. But I get it. I get this point of view too.
Well, I hope everything goes smoothly and you guys manage to catch up.

(This can probably be closed now by the way)

@kofaysi
Copy link
Contributor

kofaysi commented Jan 18, 2019

I have tested this item ✅ successfully on 0973b87

Tested with currency abbreviations. Worked well.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23499.

@rdeutz
Copy link
Contributor

rdeutz commented Jan 19, 2019

@wilsonge rebase on 4.0-dev ?

@rdeutz rdeutz removed the RTC This Pull Request is Ready To Commit label Jan 19, 2019
@brianteeman
Copy link
Contributor Author

@HLeithner @wilsonge will this be accepted for J3 or should I refactor for J4

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 16, 2019
@HLeithner
Copy link
Member

@brianteeman

Please migrate to j4 and please if its possible for you add post- and prefix maybe as selectbox.

@brianteeman
Copy link
Contributor Author

I will rebase for j4 but i will not add the ability to only have either a prefix or a suffix
That places a limitation on the usage that is not required

@HLeithner
Copy link
Member

Add both?

@AndySDH
Copy link
Contributor

AndySDH commented Mar 18, 2019

See my comment for inspiration :)
#23499 (comment)

@brianteeman
Copy link
Contributor Author

yes i will do both but not have a switch

@wilsonge
Copy link
Contributor

Once this is migrated to 4.0 branch this is approved (in case another maintainer gets to it before me)

brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Mar 18, 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.

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.
@brianteeman
Copy link
Contributor Author

Closed see #24232

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 18, 2019
@brianteeman brianteeman deleted the field-units branch March 18, 2019 16:31
wilsonge pushed a commit that referenced this pull request Apr 23, 2019
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.
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.
@brianteeman brianteeman mentioned this pull request Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet