Problem/Motivation

Due to a lot of changes in D8 core since the last the release the module is not working anymore.

Proposed resolution

  • rewrite module to latest D8 standards
  • use service instead of helper functions
  • custom element type for hms field

Remaining tasks

  • Community review
  • Clarify, why we need the javascript in the module - the theme function is never called with the "is running" => 1 parameter

While developing a small D8 page for our running club, i'll try to fix the issues mentioned above and create a rewritten version of the module for D8.

CommentFileSizeAuthor
#15 interdiff-2608886-7-15.txt928 bytesandersen_ti
#15 hms_field_d8_rewrite-2608886-15.patch59.67 KBandersen_ti
#12 hms_field_d8_rewrite-2608886-12.patch3.87 KBbugeyed
#7 hms_field_d8_rewrite-2608886-7.patch59.65 KBsteffenr
#4 hms_field_d8_rewrite-2608886-4.patch59.41 KBsteffenr

Comments

SteffenR created an issue. See original summary.

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
StatusFileSize
new hms_field_d8_rewrite-2608886-4.patch59.41 KB

Attached you find a big patch of the module rewrite - tested with drupal 8 RC3. I used drupal console module to build the sceleton for the files and used the existing functions in the new structure.
All the helper functions where moved into the HMSService and can be used via dependency injection or just by calling directly via drupal service container.

Validation partly working for now
For now the field element validation won't work correctly, cause i could not figure out how to handle the valueCallback function in conjunction with empty field content.
If you enter non matching values in the form element it'll save, but won't output the content. Maybe someone can have a look at the validateHMS and valueCallback function to fix the issue.

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Assigned: steffenr » Unassigned
Status: Active » Needs review
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
StatusFileSize
new hms_field_d8_rewrite-2608886-7.patch59.65 KB

After a short break and a recap of the validation / value callback methods i got the validation working ;)
Finally i fixed the valueCallback / validateHMS methods and got it working. Even empty values are now handled correctly. The latest patch is rerolled against the latest DEV version of the module.

Please review the patch and provide feedback.

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
steffenr’s picture

steffenr
Primary language German
Location Bremen
commented
Issue summary: View changes
jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented
Status: Needs review » Reviewed & tested by the community

Working well for me, so far.
Thanks for the rewrite, SteffenR!

bugeyed’s picture

bugeyed commented
StatusFileSize
new hms_field_d8_rewrite-2608886-12.patch3.87 KB

SteffenR, very much appreciate the rewrite/port. I've made a slight modification to HMSFieldWidget.php to output the form element as type="time" to make it a little more end-user friendly.

This is my first time uploading a file, please let me know if I've done so correctly.

The patch file is the file: hms_field/src/Plugin/Field/FieldWidget/HMSFieldWidget.php explicitly with a small change to line 107

'#attributes' => array('class' => array('hms-field'),

to

'#attributes' => array('class' => array('hms-field'), 'type'=>'time'),

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented

@bugeyed
Thanks for providing feedback and adding some extra
Please have a look at the d.o guidelines, how to create a patch. Right now you have copied the whole file and made this little change.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Can we get the patch commited?
Also it makes sense to create a beta release after that

andersen_ti’s picture

andersen_ti commented
StatusFileSize
new hms_field_d8_rewrite-2608886-15.patch59.67 KB
new interdiff-2608886-7-15.txt928 bytes

7 and 12 patches combined into one

andypost’s picture

andypost
he/him
Primary language Russian
commented

Please accept this patch and roll out beta release

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented

@Andypost
Unfortunately i could not get any response by the current module maintainer. I already opened an issue according this fact.
Maybe we should try contacting him again - would be great to have a beta release for the D8 branch of the module.

andypost’s picture

andypost
he/him
Primary language Russian
commented

There's a procedure for that case https://www.drupal.org/node/251466

  • jura1985 committed 744cee3 on 8.x-1.x 
    Issue #2608886 by SteffenR, andersen_ti, bugeyed: Branch 8.x-1.x-dev no...
ws.agency’s picture

ws.agency commented

Thank you all for the patches and review. We took maintainership of the module and just released beta 1 for D8.

https://www.drupal.org/project/hms_field/releases/8.x-1.0-beta1

ws.agency’s picture

ws.agency commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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