Clarify how WeekViewStyle and DayViewStyleBuilder are combined together

[luistrivelatto]

Currently, WeekView has two styling params: style and dayViewStyleBuilder. But it is not clear how these two are combined together, opening room for some ambiguity: which one takes priority, when both have set the same param? Also, a couple of params of dayViewStyleBuilder are ignored in the WeekView context, and there are some params which are present in dayViewStyleBuilder but not in WeekViewStyle. Here's what I've compiled:

Params present only in WeekViewStyle

• dayViewWidth
• dayViewSeparatorWidth
• dayViewSeparatorColor

These are ok, they only make sense in WeekView and there's no confusion.

Params present only in DayViewStyle

• backgroundColor
• backgroundRulesColor
• currentTimeRuleColor
• currentTimeRuleHeight
• currentTimeCircleColor
• currentTimeCircleRadius
• currentTimeCirclePosition

I believe all these params apply to WeekView too, but can't be set through DayViewStyle. So this is somewhat confusing (since if the user wants to set these params, he needs to add a DayViewStyleBuilder, whereas other params can be set through WeekViewStyle).

Params present in both, but their value in DayViewStyleBuilder is never used, only the value from WeekViewStyle is used

• hourRowHeight
• dayBarHeight
• hoursColumnWidth

These are overridden by WeekView to be 0 for each DayView. I think it's ok to ignore these values from DayViewStyleBuilder, since in the WeekView they only make sense in the group as a whole. But this should be indicated through docs/code (for example, by settings asserts to check that DayViewStyleBuilder doesn't try setting these 3 values).

Remaining params present in both

• dateFormatter
• timeFormatter
• dayBarTextStyle
• dayBarBackgroundColor
• hoursColumnTextStyle
• hoursColumnBackgroundColor

When these params are present in both style and dayViewStyleBuilder, the value from style is always given priority. I think it would be better to give priority to dayViewStyleBuilder, allowing for example to customize the dayBarTextStyle according to the current day, which is a feature wanted in #41.

Idea for clarifying WeekView's style
It seems to me that a good way to make things clearer would be to strip out from WeekViewStyle all the params written in the last list, leaving it like this:

WeekViewStyle(
  hourRowHeight,
  dayBarHeight,
  hoursColumnWidth,
  dayViewWidth,
  dayViewSeparatorWidth,
  dayViewSeparatorColor
)

Then the default (and only) way to customize style which is not WeekView-specific would be to set a DayViewStyleBuilder. Also, the 3 first params (hourRowHeight, etc), which would still be present in DayViewStyle, could be assert-checked that they are not set in DayViewStyleBuilder.

This would:

• fix ambiguity of params present in both styles (no param can be set in both styles simultaneously)
• allow to customize params which are DayView-specific according to the date (e.g. custom style for current day)
• remove possible confusion from setting hourRowHeight, dayBarHeight and hoursColumnWidth in DayViewStyleBuilder (would throw an assertion failure)
• make it so that, if new DayView-specific styling options are added which also make sense at WeekView level, they only need to be added in one place (currently, they should be added at both styles for consistency)

I understand this would not be a trivial change (plus it would be a breaking one), but I believe the API would be cleaner as a result. Thoughts? :)

[Skyost]

I agree with you. The current system allows to separate the style part from the code part, but it may be a bit messy. Here's what we need to do :

• Refactor the WeekViewStyle class.
• Review and merge Customize TextStyle of current day on day bar & the first day visible on WeekView #41.
• Test if everything is working as expected.

[luistrivelatto]

Removing the WeekViewStyle and adding the params to WeekView should work indeed. But I liked your idea of isolating the style (height/width/colors) from other more behaviour-ish parameters, so I'd be favorable to keeping the class, but just reducing the intersection with DayViewStyle params. It would also keep consistency with patterns in Flutter widgets (such as the 'decoration' param in Container). What do you think?

[Skyost]

What do you think?

I agree. I just thought about it briefly to be honest 😄