Skip to content

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

Merged
bembelimen merged 1 commit intojoomla:4.1-devfrom
Digital-Peak:j4/fields/form-layout
Jan 6, 2022
Merged

[4.1]Add form layout option to custom fields#36551
bembelimen merged 1 commit intojoomla:4.1-devfrom
Digital-Peak:j4/fields/form-layout

Conversation

@laoneo
Copy link
Copy Markdown
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
Copy Markdown
Contributor

brianteeman commented Jan 3, 2022

Is this small bug part of this pr?

radio

@brianteeman
Copy link
Copy Markdown
Contributor

Otherwise everything looks great to me

@laoneo
Copy link
Copy Markdown
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
Copy Markdown
Contributor

ok

@brianteeman
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

laoneo commented Jan 6, 2022

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

@AndySDH
Copy link
Copy Markdown
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
Copy Markdown
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