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

[4.1]Add form layout option to custom fields #36551

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Jan 3, 2022

Pull Request for Issue #36548.

Summary of Changes

Makes the form layout for custom field configurable. Actually only for the following fields:

  • list
  • sql
  • radio

Any suggestions for language strings?

Testing Instructions

  • Create a list custom field where multiple is set to yes
  • Change the form layout to enhanced list in the options tab
  • Edit an article

Actual result BEFORE applying this Pull Request

The list is rendered with the default select HTML Element.

Expected result AFTER applying this Pull Request

The list is rendered with the choices.js script.

@brianteeman
Copy link
Contributor

brianteeman commented Jan 3, 2022

Is this small bug part of this pr?

radio

@brianteeman
Copy link
Contributor

Otherwise everything looks great to me

@laoneo
Copy link
Member Author

laoneo commented Jan 3, 2022

No, this looks like an init issue on the JS part which I didn't touch. But could also not reproduce the issue on my end.

@brianteeman
Copy link
Contributor

ok

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 5da3d2a


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

1 similar comment
@Shubhamverma2796
Copy link
Contributor

I have tested this item ✅ successfully on 5da3d2a


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

@Quy
Copy link
Contributor

Quy commented Jan 3, 2022

RTC


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

@Quy Quy removed the Language Change This is for Translators label Jan 3, 2022
@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 3, 2022
@crystalenka
Copy link
Member

I have tested this item ✅ successfully on 5da3d2a

Beautiful!!! Thanks so much, and also for including the radio layout!


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

@crystalenka
Copy link
Member

I reproduced part of Brian's issue. It's because the layout expects the values to be in the order of No - Yes; if they are flipped, it doesn't respect that. I'm wondering if there is any way to detect the values of 0/1 or true/false or yes/no and make it a bit 'smarter'? I'm not sure this is something that would prevent merging the PR, but wanted to clarify the bug.


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

@laoneo
Copy link
Member Author

laoneo commented Jan 3, 2022

Thanks for clarification. When the order mathers, then we need to mention this in the docs.

If this should be handled on the code side, then I would do it in another pr.

@bembelimen bembelimen merged commit ac2afac into joomla:4.1-dev Jan 6, 2022
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators and removed RTC This Pull Request is Ready To Commit labels Jan 6, 2022
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.1 milestone Jan 6, 2022
@laoneo laoneo deleted the j4/fields/form-layout branch January 6, 2022 12:07
@laoneo
Copy link
Member Author

laoneo commented Jan 6, 2022

Can you guys also test #36553 which does the same for the color field.

@AndySDH
Copy link
Contributor

AndySDH commented Feb 26, 2022

@laoneo I just ran into this and I noticed this is the wrong placement for this setting.

The "Options" tab is dedicated to global settings that are shared by all field types. It's not the place to add type-specific settings.

Options tied to specific types of fields should be added in the first "General" tab. This is what has always been done for all field types and field settings.

So this new "Form Layout" should be moved under the "General" tab for all the fields in which you added it. For example, under the "Multiple" setting:

image

But: Do we even need this setting at all?

I would argue: do we really need a setting? The other "raw" layout is ugly and unusable. I don't think anyone will ever want to use it.

The new one it's just better, and it's similar to what he had in J3.

I personally would just implement the "enhanced select" by default, and get rid of the other layout altogether. Like we had in J3. What do you think?

@laoneo
Copy link
Member Author

laoneo commented Feb 28, 2022

Please open a new issue, so it can be discussed properly as this pr is closed.

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.

8 participants