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.

Sign up for GitHub

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

Jump to bottom

Fix the issue where the timeline.startHour was ignored #353

Merged
merged 2 commits into from
Jun 27, 2024
Merged

Fix the issue where the timeline.startHour was ignored #353

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0fe4db
Fix the issue where the `timeline.startHour` was ignored
rakuyoMo Jun 3, 2024
d3d869e
Merge branch 'master' into fix/calculateChangingTime
rakuyoMo Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/KVKCalendar/Timeline+Extension.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ extension TimelineView: EventDelegate {
let firstY = time.frame.origin.y - (calculatedTimeY + style.timeline.heightTime)
let percent = (pointY - firstY) / (calculatedTimeY + style.timeline.heightTime)
let newMinute = Int(60.0 * percent)
let newHour = time.tag - 1
let newHour = time.tag - 1 + style.timeline.startHour
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, did you test this case? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, did you test this case? 🤔

Yes, I have tested it.

Logically speaking, as shown in the code below, the time tag starts at 0, which does not really reflect the time corresponding to the label.

KVKCalendar/Sources/KVKCalendar/Timeline+Extension.swift

Line 339 in 983d4c5

time.tag = idx - start

I still don't understand why the above code needs to subtract start. Because the time tag is used in many places, I used a method with less impact, as shown in this PR.

Perhaps after understanding the above reasons for modification, you will have a more correct way to modify it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My current testing is not comprehensive, but I will be able to do some more comprehensive testing next week. Based on my current test results, there is nothing wrong with this modification.

Copy link
Owner

Choose a reason for hiding this comment

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

I'll check this case before merging. Thanks for your explanation 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kvyatkovskys I have tested it in my project and it is necessary

return (newHour, newMinute)
}

Expand Down