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

fix: resolve schedule rendering issues on month view (#646) #647

Merged
merged 7 commits into from
Sep 3, 2020

Conversation

jungeun-cho
Copy link
Contributor

@jungeun-cho jungeun-cho commented Jul 1, 2020

Please check if the PR fulfills these requirements

  • It's submitted to right branch according to our branching model
  • It's right issue type on 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 description for the breaking change

Description

AS-IS TO-BE
tui-calendar-issue-646-1 tui-calendar-issue-646-1-tobe
AS-IS TO-BE
แ„‰แ…ณแ„แ…ณแ„…แ…ตแ†ซแ„‰แ…ฃแ†บ 2020-07-01 10 48 35 แ„‰แ…ณแ„แ…ณแ„…แ…ตแ†ซแ„‰แ…ฃแ†บ 2020-07-01 10 49 19
  1. on monthly view,
  2. When adding a new schedule with the default schedule creation popup
  3. enter start time 2020-07-23 00:00, end time 2020-07-23 00:00 and 'All day' checked
    แ„‰แ…ณแ„แ…ณแ„…แ…ตแ†ซแ„‰แ…ฃแ†บ 2020-07-23 20 36 29
AS-IS TO-BE
แ„‰แ…ณแ„แ…ณแ„…แ…ตแ†ซแ„‰แ…ฃแ†บ 2020-07-23 20 39 03 แ„‰แ…ณแ„แ…ณแ„…แ…ตแ†ซแ„‰แ…ฃแ†บ 2020-07-23 20 42 22

Thank you for your contribution to TOAST UI product. ๐ŸŽ‰ ๐Ÿ˜˜ โœจ

@jungeun-cho
Copy link
Contributor Author

@lja1018

Copy link
Contributor

@dongsik-yoo dongsik-yoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[07/01] Reviewed.
In case of issue No.1, dragging that event must be tested with same manner(resizing it). When dragging that event, what if start and end hours are changed? It might be another bug.

src/js/controller/viewMixin/core.js Outdated Show resolved Hide resolved
src/js/controller/viewMixin/core.js Outdated Show resolved Hide resolved
src/js/handler/month/core.js Outdated Show resolved Hide resolved
Copy link
Contributor

@lja1018 lja1018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[07/01] Reviewed.

src/js/controller/viewMixin/core.js Outdated Show resolved Hide resolved
@jungeun-cho
Copy link
Contributor Author

ok to test

@nhn nhn deleted a comment from swtalk Jul 23, 2020
Copy link
Contributor

@lja1018 lja1018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[07/27] Reviewed.

Comment on lines +679 to +689
var start = isAllDay ? datetime.start(startDate) : startDate;
var end = isAllDay ? datetime.renderEnd(startDate, endDate) : endDate;

/**
* @typedef {object} RangeDate
* @property {TZDate} start start time
* @property {TZDate} end end time
*/
return {
start: new TZDate(startDate),
end: new TZDate(endDate)
start: new TZDate(start),
end: new TZDate(end)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about doing this

return {
  start: new TZDate(isAllDay ? datetime.start(startDate) : startDate),
  end: new TZDate(isAllDay ? datetime.renderEnd(startDate, endDate) : endDate)
};

@nhn nhn deleted a comment from swtalk Jul 27, 2020
@dilyanpalauzov
Copy link

dilyanpalauzov commented Jul 27, 2020

I have two theoretical questions, for which I have not done tests:

  • if an event ends at midnight, e.g. 2020-10-05T00-00-00, does the above code, and all other code, hide the event from 5th October?

  • a schedule can be isAllday, or its category can be 'time'. If the duration is more than 24h, controller/base.js groupFunc handles the schedule as being allday, sometimes. When is an event, lasting at least 24h, handled as allday and when not, if its isAllday property is not set?

@nhn nhn deleted a comment from swtalk Jul 27, 2020
@stale
Copy link

stale bot commented Aug 26, 2020

This issue has been automatically marked as inactive because there hasnโ€™t been much going on it lately. It is going to be closed after 7 days. Thanks!

@stale stale bot added the inactive label Aug 26, 2020
@stale
Copy link

stale bot commented Sep 2, 2020

This issue will be closed due to inactivity. Thanks for your contribution!

@stale stale bot closed this Sep 2, 2020
@jungeun-cho jungeun-cho reopened this Sep 3, 2020
@stale stale bot removed the inactive label Sep 3, 2020
@nhn nhn deleted a comment from swtalk Sep 3, 2020
@nhn nhn deleted a comment from swtalk Sep 3, 2020
@jungeun-cho jungeun-cho merged commit d056190 into master Sep 3, 2020
@nhn nhn deleted a comment from swtalk Sep 3, 2020
@nhn nhn deleted a comment from swtalk Sep 3, 2020
@swtalk
Copy link

swtalk commented Sep 3, 2020 via email

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.

change an all-day schedule to a new single day failed.
5 participants