search

craftcms / store-hours

Manage business hours with Craft CMS.
https://plugins.craftcms.com/store-hours
MIT License
61 stars 15 forks source link

defaultWeekStartDay Support #11

Closed howells closed 7 years ago

howells commented 7 years ago

I have my defaultWeekStartDay set to Monday, but the plugin starts on Sunday - shouldn't it honour whatever the default is set to?

brandonkelly commented 7 years ago

That config setting, and users’ Week Start Day settings, are currently only used when displaying the calendar popups on date inputs. But we could update Store Hours to respect it as well, yes.

howells commented 7 years ago

@brandonkelly that would be awesome :-) I tried to work out how to do it for a PR but failed miserably

brandonkelly commented 7 years ago

Haha that’s ok, we’ll get to it.

howells commented 7 years ago

Thanks for making this update! But it seems the release hasn't been made so the 1.2 update shown in the CP doesn't link to anywhere. (https://codeload.github.com/pixelandtonic/StoreHours/zip/1.2)

angrybrad commented 7 years ago

Should be resolved now!

gregorydavidjenkins commented 7 years ago

Doesn't this break the output though?

If you use the recommended output method of creating a days array, the accuracy of the information being outputted changes depending on who the last person to update the entry was.

If USER A has their week starting on Monday and saves the entry it breaks the output, if you change your array to reflect that and USER B (whose week starts on Sunday) updates the entry, it is broken again.

Do I need to check the defaultWeekStartDay of the last user to update the page and create my array from there? I think the name of the day of the week should be built into the data coming from the field.

brandonkelly commented 7 years ago

@gregorydavidjenkins That change shouldn't have affected the way the data is stored, with the first day being Sunday. Are you just speculating or have you actually found the data to be ordered differently?

brandonkelly commented 7 years ago

(and if the latter, please open a new issue so we can track it separately from the original feature request.)