Problem/Motivation
There are a few problems when creating homeboxes directly:
1.
If we press on the "Add Homebox / Preset" button on "/admin/structure/homebox_type/{homebox_type}/homeboxes" "isPreset" is enabled by default, which is quite confusing. We should differ between "Add Homebox" and "Add Preset" buttons and prebuild the form based on url parameters
2.
When going to the "/admin/homebox/add/{homebox_type}" route, the following error appears:
Warning: Undefined array key 0 in Drupal\homebox\Plugin\Field\FieldWidget\PortletWidget->formElement() (line 84 of modules/custom/homebox/src/Plugin/Field/FieldWidget/PortletWidget.php).
3. When adding a preset, we should manually suffix the preset number to the label. The automatically generated preset on homebox type generation is called "{homebox_type->label} Preset 0". So any other created presets should be named preset 1..2....5 etc..
Meaning we should load all preset homeboxes of the defined homebox_type, count them and add the appropriate number to prepopulate the add form label.
Steps to reproduce
Proposed resolution
Fix the problems.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork homebox-3439626
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedAll done, please review!
Comment #3
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedComment #5
AnybodyNice work, but I'm not really sure the context parameter is the best possible solution.
I'm a bit more in favor of using inheritance and two different form classes inheritng from the base class.
That's typically the better polymorphysm pattern instead of if's that tend to grow by time and code becomes ugly.
Furthermore, in the current implementation you have to ensure the context parameter is always present, which is also a sign of improvable design. In that case two separate same-based routes / URLs could do the job better without edge-cases like
which for routes the system would handle itself by design (404)
Comment #6
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedAll done, final review pls.
Comment #7
AnybodyLGTM. #5 will be documented and solved in a follow-up later. @Grevil will link it in both directions.
Comment #9
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedThanks!
Comment #10
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedComment #11
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedHere is the follow up issue: #3440217: Instead of passing a context in "HomeboxForm", divide the Form into two seperate forms.