Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: fixed-week behavior #75

Merged
merged 15 commits into from
May 21, 2021
Merged

Conversation

pdpino
Copy link
Collaborator

@pdpino pdpino commented Apr 15, 2021

Fixes #74.

I created a FixedWeekView component, to be used as a fixed timetable showing one week.

Some code is repeated from WeekView, which is not ideal (though I moved most functions to utils.js). I created it this way for two main reasons:

  1. If the WeekView component were to be reused, there would be useless virtualized-lists within the component, which is not optimal
  2. In the future both components may differ more, and may be useful to have them separated.

If we wanted to avoid code repetition with a new component, instead of creating FixedWeekView as a component of its own, we could add a prop to WeekView such as fixedHorizontally={true/false} (or other name), which would disable horizontal scrolling, plus ensuring the week is displayed starting from monday.

@hoangnm let me know if you prefer a different solution! The PR will be a draft for now.

@pdpino pdpino changed the title Feature/fixed week Feature FixedWeekView component Apr 16, 2021
@pdpino pdpino changed the title Feature FixedWeekView component Feature: FixedWeekView component Apr 16, 2021
@hoangnm
Copy link
Owner

hoangnm commented Apr 16, 2021

Hi @pdpino, thanks again for your help! I think we can have the prop to prevent horizontal scrolling at the moment unless we have more use cases for the FixedWeekView.

@pdpino
Copy link
Collaborator Author

pdpino commented Apr 16, 2021

Ok, sounds good

I will refactor this PR soon!

@pdpino pdpino marked this pull request as ready for review April 17, 2021 18:22
@pdpino
Copy link
Collaborator Author

pdpino commented Apr 17, 2021

@hoangnm I updated the code and marked as ready:

  • Added the prop fixedHorizontally and function createFixedWeekDate(), to support the fixed-week behavior
  • Deleted the extra FixedWeekView component
  • I updated the README with an example of how to use the component as fixed
  • I did move some functions to utils, to make the WeekView component smaller (should be easier to mantain) (will do this in a separate PR)

@pdpino pdpino changed the title Feature: FixedWeekView component Feature: fixed-week behavior Apr 17, 2021
@pdpino pdpino added the type: feature enhancements or new features label May 16, 2021
@hoangnm
Copy link
Owner

hoangnm commented May 21, 2021

Great, I'm merging now.

@hoangnm hoangnm merged commit b3e9760 into hoangnm:master May 21, 2021
@pdpino pdpino deleted the feature/fixed-week branch May 22, 2021 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature enhancements or new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to disable swiping?
3 participants