search

Quivr / iOS-Week-View

QVRWeekView is a framework which provides a calendar view that can be customized to display between 1 to 7 days in both portrait and landscape mode. Includes customization features to customize colours, fonts and sizes.
MIT License
39 stars 21 forks source link

Do not use global LayoutVariables #9

Closed lesyk closed 4 years ago

lesyk commented 6 years ago

Any updates?

reilem commented 6 years ago

Didn't receive a notification about this so haven't had a proper look at it yet. But it sounds like a good idea. :)

lesyk commented 6 years ago

@reilem More fixes.

lesyk commented 6 years ago

@reilem 704ec4e is pretty bad way to fix it, but for not I think it will work. Definitely the way how all day events are build need to be re-written to autolayout.

reilem commented 6 years ago

I've finally had the chance to look at this in detail. Sorry for the delay. I like the idea of removing the "global static variables" method of storing layout fields. However attaching them all to DayScrollView isn't a great solution. Since some of those fields, such as top bar height are not really the dayScrollView's responsibility. Those are more related to the WeekView because the weekView is the one that actually stores the topBar.

Seeing as right now many of the layout variables are being stored double (once in LayoutVariables and once in the View themselves). It might be best to just eliminate the LayoutVariables class entirely and try to adopt a completely different strategy.

I'm going to start working on a fix for a bug surrounding the all day events causing screen jitter. Perhaps while I fix this I can start thinking about a different way to implement the LayoutVariables class.

reilem commented 4 years ago

I finally got round to fixing this issue. Thank you for this PR @lesyk, it gave me some inspiration while making PR #22. All global variables should be removed, and I shoved all the variables into classes that needed them the most. If you find anything weird or could be better feel free to let me know. I will be closing this PR for now. Thanks again!