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

refactor: Replace 7 by WEEK_DAYS #1225

Merged
merged 1 commit into from
Jul 27, 2022
Merged

refactor: Replace 7 by WEEK_DAYS #1225

merged 1 commit into from
Jul 27, 2022

Conversation

HansSchouten
Copy link
Contributor

@HansSchouten HansSchouten commented Jul 23, 2022

Please check if the PR fulfills these requirements

  • It's the right issue type on the title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has a description of the breaking change

Description

I am using a calendar week view with 4 weeks of events in one long row. For this I needed to increase WEEK_DAYS. However, I found out that in this line of code the number of days per week is still hardcoded instead of relying on the WEEK_DAYS constant.

afbeelding


Thank you for your contribution to TOAST UI product. πŸŽ‰ 😘 ✨

I am using a calendar view of 4 weeks in one long row. For this I needed to increase WEEK_DAYS. However, I found out that in this line of code the number of days per week is still hardcoded instead of relying on the WEEK_DAYS constant.
@HansSchouten HansSchouten changed the title Replace 7 by WEEK_DAYS refactor: Replace 7 by WEEK_DAYS Jul 23, 2022
@adhrinae
Copy link
Contributor

@HansSchouten
Thanks for pointing it out.

So did you fork the repo and modified the WEEK_DAYS variable and it shows several weeks in a row?

@HansSchouten
Copy link
Contributor Author

HansSchouten commented Jul 25, 2022

Yes that is correct, I needed an opensource version of Bryntum Scheduler but found out these do not exist.
So I decided a slightly modified TUI Calendar would also do the job.
I cloned and only changed one line to set WEEK_DAYS to 28.
I love how little was actually needed πŸ˜ƒ

And then I enabled the weekly view and applied the following week configuration:

week: {
    showTimezoneCollapseButton: false,
    timezonesCollapsed: true,
    narrowWeekend: false,
    taskView: false,
}

And next I applied the following style:

  .toastui-vue-calendar {
      min-width: 2000px;
  }
  .toastui-calendar-time,
  .toastui-calendar-week-view-day-names,
  .toastui-calendar-panel-resizer {
      display: none;
  }
  .toastui-vue-calendar:first-of-type {
      .toastui-calendar-week-view-day-names {
          display: block;
      }
  }
  .toastui-calendar-day-name-container {
      margin-left: 305px !important;
  }
  .toastui-calendar-panel-title {
      width: 300px !important;
      box-sizing: border-box;
      display: table;
  }
  .toastui-calendar-template-alldayTitle {
      display: table-cell;
      vertical-align: middle;
      text-align: right;
      white-space: normal;
      font-size: 13px;
      padding-right: 8px;
  }
  .toastui-vue-calendar:nth-of-type(even) > div {
      background: #f8f9fa !important;
  }
  .toastui-calendar-panel-grid {
      border-right: 1px solid #eee !important;
  }

And not all of this is actually needed, parts of it is my personal preference.
Next I rendered multiple calendars below eachother with Vue using a v-for (in this case each row represents the reservations for a different holiday rental).
And that is how I obtained the screenshot, and it is working perfectly for my usecase.

However, instead of modifying the WEEK_DAYS constant and use my separate repo clone is there a way to make a setting for this?
That would be even more awesome!

@adhrinae
Copy link
Contributor

@HansSchouten

So I decided a slightly modified TUI Calendar would also do the job.
I cloned and only changed one line to set WEEK_DAYS to 28.
I love how little was actually needed πŸ˜ƒ

Sounds amazing πŸ‘

However, instead of modifying the WEEK_DAYS constant and use my separate repo clone is there a way to make a setting for this?

We don't offer those features currently, so I think your approach is pretty clever.

There are several requests such as drawing several weeks in a row. We haven't had an idea to implement this feature yet, however, your approach gave me an idea.

Maybe we can add an option to set WEEK_DAYS like this? Of course, some other components & modules should be changed with it.

@HansSchouten
Copy link
Contributor Author

Ok, glad to see it could be of help to others as well.
Yes a setting to configure WEEK_DAYS would be a great option for this, however I don't see how to pass this on to datetime.ts.

@adhrinae
Copy link
Contributor

adhrinae commented Jul 27, 2022

@HansSchouten

Options are stored in the state management store for components. and every component can get those options through the useStore custom hook.

So components should bring the WEEK_DAYS variable from the option and pass it into controller/helper functions calculating styles, layouts, etc.

I assume that it needs major refactoring, so I'll just consider it later. At least I can introduce this conversation to the discussion board for others.

@HansSchouten
Copy link
Contributor Author

Thanks for the explaination, for now it works for me as well, but great to include it in future conversations.

@adhrinae adhrinae merged commit 8a0c6ab into nhn:main Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants