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

[5.0] Add alternative field labels (+ a new attribute) for the Status field #35743

Closed
wants to merge 10 commits into from

Conversation

ditsuke
Copy link
Contributor

@ditsuke ditsuke commented Oct 4, 2021

Summary of Changes

Many extensions currently use custom status fields (PluginStatusField, RedirectStatusField, TaskStateField (from #35143)), mainly because it makes more contextual sense to use Enabled/Disabled as states at some places instead of Published/Unpublished.
This also increases scope for inconsistencies which can be seen as an example in the forms for com_workflows (reference #35056).

This PR adds alternative labels the status field which can be used with the alt_labels field attribute. This can be used in conjunction with optionsFilter at places where a list field or a custom field might be needed right now.

For example, if you want to support the enabled, disabled and trashed state for an item, you could use:

<field 
	name="state"
	type"status" 
	label="JSTATUS"
	optionsFilter="-2, 0, 1"
	alt_labels="true"
/> 

Right now, you'd have to contend with a custom field, a list field everywhere or a language override (#35743 (comment)).

Testing Instructions

You can test that StatusField (field type="status") behaves the same as before in all existing forms.
I can also add the alt_labels attribute to the workflow forms and some more places in this PR so those could be tested here and perhaps some redundant field classes removed.

Actual result BEFORE applying this Pull Request

StatusField only supports "publish" type labels.

Expected result AFTER applying this Pull Request

StatusField supports alternative state labels needed by a number of existing core extensions.

Documentation Changes Required

Update to the Form Fields/StatusField documentation on JDocs.

@brianteeman
Copy link
Contributor

Could you give an example please of where this would be used

@ditsuke
Copy link
Contributor Author

ditsuke commented Oct 4, 2021

Could you give an example please of where this would be used

Workflow forms (I linked a PR for reference), replacing PluginStatusField, RedirectStatusField and TaskStatusField (from Scheduler).

@brianteeman
Copy link
Contributor

Personally I have always done this with just a language string

For example (together with your fix for workflows) it will look like this
image

Just by adding two language string overrides to com_workflows.ini
image

@ditsuke
Copy link
Contributor Author

ditsuke commented Oct 4, 2021

Personally I have always done this with just a language string

For example (together with your fix for workflows) it will look like this image

Just by adding two language string overrides to com_workflows.ini image

That's a good solution! I think this change should still be worth considering as it might be easier perhaps to use an attribute for the safe effect as its used in quite a few places.

@ditsuke ditsuke force-pushed the enhance-status-field branch 2 times, most recently from c002bd2 to df66317 Compare October 4, 2021 10:11
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester Unit/System Tests labels Oct 4, 2021
@ditsuke ditsuke changed the base branch from 4.0-dev to 4.1-dev October 4, 2021 10:11
@joomla-cms-bot joomla-cms-bot added PR-4.1-dev and removed Language Change This is for Translators Unit/System Tests NPM Resource Changed This Pull Request can't be tested by Patchtester labels Oct 4, 2021
Adds alternative labels with JDISABLED/JENABLED for StatusField which
can be used with the `alt_labels="true"` field attribute.
These alternative labels make more sense in many contexts and
effectively cover for custom fields used by many extensions when used in
conjunction with the `optionsFilter` attribute.
Allows again the form to define additional options retaining the
original behavior.
StatusField now extends PredefinedlistField again, reverting a potential
B/C break.

Refs: @bembelimen
@ditsuke
Copy link
Contributor Author

ditsuke commented Oct 4, 2021

Rebased to 4.1-dev

@richard67 richard67 changed the title [4.0] Add alternative field labels (+ a new attribute) for the Status field [4.1] Add alternative field labels (+ a new attribute) for the Status field Nov 13, 2021
@fancyFranci
Copy link
Contributor

I have tested this item ✅ successfully on 255d840

I tested the alt_labels attribute in several filters like categories and the patch worked. Enabled and Disabled showed up.


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

@HLeithner HLeithner changed the base branch from 4.1-dev to 4.2-dev June 27, 2022 13:07
@HLeithner
Copy link
Member

This pull request has automatically rebased to 4.2-dev.

@joomla-bot
Copy link
Contributor

This pull requests has been automatically converted to the PSR-12 coding standard.

@chmst chmst added the Feature label Oct 22, 2022
@HLeithner HLeithner removed the psr12 label Oct 23, 2022
@HLeithner HLeithner changed the base branch from 4.2-dev to 5.0-dev May 2, 2023 16:42
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.0-dev.

@brianteeman
Copy link
Contributor

Looking at this again and I realise now the benefits. Together with the optionsFilter this PR would allow us to remove several fields as they would become redundant.

@brianteeman
Copy link
Contributor

could someone change the title to say [5.0] instead of 4.1 please

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on bd41106


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

@ditsuke ditsuke changed the title [4.1] Add alternative field labels (+ a new attribute) for the Status field [5.0] Add alternative field labels (+ a new attribute) for the Status field Jul 14, 2023
@HLeithner
Copy link
Member

The proper way would be to create an addition field with other default values, then you don't need an extra attribute.

Something like StateField instead of StatusField.

@ceford
Copy link
Contributor

ceford commented Sep 11, 2023

I have tested this item ✅ successfully on bd41106

Tested on 5.0.0-beta2-dev. I added an extra dummy Option to see it in the filter list and also stepped through the applied code. I added two lines from the state field to the articles published field to see the effect of the alt_labels option. All works.


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

@richard67
Copy link
Member

Although it has 2 successful human tests I don't set it to RTC because it has a conflict in file "libraries/src/Form/Field/StatusField.php".

And @HLeithner 's comment above suggest a different solution.

@richard67 richard67 added the RMDQ ReleaseManagerDecisionQueue label Sep 12, 2023
@HLeithner
Copy link
Member

as said own field instead of additonal attribute please. I'm closing this but can be reopend when we get it's own field or a more flexible way to change the label of the options (maybe swapable language constants)

@HLeithner HLeithner closed this Sep 12, 2023
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.

None yet