Problem

Currently, there is an option to Automatically generate the label if the label field is left empty.
Would be good to also have the ability to pre populate field with pattern value, but and keep it required.

Solution

Add new option PREFILLED Automatically prefill the label

CommentFileSizeAuthor
#3 2.png90.06 KBrenatog
#3 1.png88.06 KBrenatog
#2 auto_entitylabel-prefilled_option-2896979-2.patch6.82 KBribel

Comments

ribel created an issue. See original summary.

ribel’s picture

ribel
Primary language Ukrainian
Location 🇺🇦Lviv
commented
Status: Active » Needs review
Related issues: +#2848128: Integration with inline entity form
StatusFileSize
new auto_entitylabel-prefilled_option-2896979-2.patch6.82 KB

Here is the patch wihich adds new option: Automatically prefill the label.
It was built on top of the patch from inline entity form issue.

I have added two new methods to AutoEntityLabelManagerInterface: getStatus() and getPattern(), which can replace autoLabelNeeded() and hasOptionalAutoLabel() methods. Also i have refactored code a little bit in auto_entitylabel_prepare_entityform(), by replacing multiple if with switch.

renatog’s picture

renatog commented
Assigned: Unassigned » renatog
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: +ciandt-contrib
StatusFileSize
new 1.png88.06 KB
new 2.png90.06 KB

Hi guys.

I applied the patch and works good for me.

Thank you very much @ribel. Good catch.

Good Work.

Regards.

renatog’s picture

renatog commented
Status: Reviewed & tested by the community » Fixed

Fixed.

Committed to the dev branch.

Thank you very much for your contribution @ribel.

Good Work.

Regards

  • RenatoG committed 49e148c on 8.x-2.x authored by ribel 
    Issue #2896979 by ribel, RenatoG: Add an option to automatically prefill...
sutharsan’s picture

sutharsan commented

@RenatoG, thanks for committing this issue, but I don't like the process how it went in. Most important, you have not credited the 3 persons who have worked on the issue that this patch was builded on. I want to see this corrected, probably with a minimal patch which credits the missing persons.

Secondly, merging two issues in one commit makes it in the future very hard to reconstruct the process that lead to a commit. I know how hard it is to push back a good contribution, but in my opinion this issue should have been postponed and rerolled after committing the 'base' issue.

Lastly, it is now unclear what code does this patch add on-top of #2848128: Integration with inline entity form and what it changes Was that actually reviewed?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

purushotam.rai’s picture

purushotam.rai commented
Status: Closed (fixed) » Needs work

Guys, I feel this ticket needs to be reopened as this option does not support token, so prefilling doesn't make sense if it can't be dynamic. The case resolved here is similar to adding helper text using field description or like placeholders.

In my opinion, it should do a lot more besides this.

Let me know your views on this.

Thanks and Regards

purushotam.rai’s picture

purushotam.rai commented

We have missed something serious here,

autoLabelNeeded function does not take care of new option "prefilled".

purushotam.rai’s picture

purushotam.rai commented
Status: Needs work » Fixed

I've added the fix to 8.x-2.x branch. Marking this issue as fixed as of now. Feel free to reopen or discuss if the issue still exists.

@Sutharsan, sorry for the misunderstanding we will definitely be more careful regarding the approach in the future. Thanks a lot for all your valuable inputs, it definitely means a lot to the community.

Thanks and Regards

sutharsan’s picture

sutharsan commented

I've added the fix to 8.x-2.x branch.

See #2922829: Prefilled Option not working fine

sutharsan’s picture

sutharsan commented
Related issues: +#2922829: Prefilled Option not working fine

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.