Using office_hours for the module name, field type, widget, formatter, FAPI element and theme functions is... confusing.

I propose to rename office_hours form element to office_hours_slot since this element provides rows for the week table.

Also I've seen the opportunity to reorder things and get rid of function _office_hours_get_field_settings().

CommentFileSizeAuthor
#3 office_hours-1913018.patch13.67 KBjonhattan
#2 office_hours-1913018.patch20.55 KBjonhattan
#1 office_hours-1913018.patch9.35 KBjonhattan

Comments

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Status: Active » Needs review
StatusFileSize
new office_hours-1913018.patch9.35 KB

-38 lines of code for clarity :)

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new office_hours-1913018.patch20.55 KB

I realized this element was named office-hours-block in some places. More renaming.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new office_hours-1913018.patch13.67 KB

Correct patch.

johnv’s picture

johnv
Primary language Dutch
commented

- This is a good idea, but renaming 'block' to 'slot' (in the css file) is not backwards compatible. Let's stick with 'block'.
- the removal of hook_field_is_empty() is just a clean-up?
- _office_hours_get_field_settings() has some provision for Profile2. Isn't that lost? (I can't find it right now, but another issue changes that part, too.)

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Status: Needs review » Needs work

I'm fine with keeping the backward compatibility for css. It can be changed in a 2.x branch.

About profile2: My guess is that problem is in other place, and this code is there to circumvent it, and it is no longer needed once the widget revamp proposed in #1168126: Default value for every day of the week is committed.

Almost the same as #1689296: Add more hours link won't work with Field collection module, but I have no evidence. I'll try to replicate the problem with profile2 and check if my stack of patches fixes it.

johnv’s picture

johnv
Primary language Dutch
commented
Title: Rename office_hours form element to office_hours_slot and get rid of _office_hours_get_field_settings() » Rename office_hours form element to office_hours_slot.
Status: Needs work » Fixed

The renaming is committed here.
It took me a while to untangle your untangle-patches. :-)
The 'get rid of _office_hours_get_field_settings()' part is not committed, mainly because of the Profile2-issue.

I'm closing this issue. You can provide a patch for the remaining part in a new issue on latest dev-version.

Status: Fixed » Closed (fixed)

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