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: issue 2536 #2545

Merged
merged 4 commits into from
Apr 15, 2024
Merged

fix: issue 2536 #2545

merged 4 commits into from
Apr 15, 2024

Conversation

forceddd
Copy link
Contributor

This is a commit to fix issue 2536 and 2529.

When I was fixing 2529, I declared slotMetrics as a global variable, which caused all DayColumn instances to use the same instance of slotMetrics. This is also the reason why bug issue 2536 appears.

In this commit, I only modified the update timing of slotMetrics.Now both issues are resolved!

2024-03-25.15.36.56.mov

@cutterbl
Copy link
Collaborator

Loading your changes in Storybook, if I go to the onSelectSlot story and begin a selection, it immediately changes it to begin at the beginning of the day up to my selection point.

@forceddd
Copy link
Contributor Author

Loading your changes in Storybook, if I go to the onSelectSlot story and begin a selection, it immediately changes it to begin at the beginning of the day up to my selection point.

This is an issue that exists in version 1.11.1 and is not caused by this commit.

@cutterbl
Copy link
Collaborator

@forceddd You are correct, but it was your previous PR that appears to have introduced this issue in the first place.

@forceddd
Copy link
Contributor Author

forceddd commented Mar 28, 2024

@cutterbl This bug should have existed a long time ago. The issue 2290 seems to be the bug described.
I will try to fix it next week, if I successfully solve this issue, I will create another PR.

@cutterbl
Copy link
Collaborator

@forceddd Thanks for looking into this. An you are partly right. That issue you pointed out was for dragging up. Now it is also doing it even if dragging down (which was not an issue before your change).

@forceddd
Copy link
Contributor Author

forceddd commented Apr 1, 2024

@cutterbl After each slotMetrics update, a new slots array will be created, but the _initialSlot object in DayColumn remains unchanged, so nextSlot becomes 0:00.
I modified nextSlot to handle this case. This video shows the performance after the modification.

2024-04-01.10.58.23.mov

@forceddd
Copy link
Contributor Author

forceddd commented Apr 1, 2024

@cutterbl This is another related issue 2546.

@forceddd
Copy link
Contributor Author

forceddd commented Apr 1, 2024

Is the margin-right here intentionally designed this way? Because when I'm selecting, there is no such spacing, but the rendered result has spacing. This makes me a little confused.
image

@nourbenamor201
Copy link

Is the margin-right here intentionally designed this way? Because when I'm selecting, there is no such spacing, but the rendered result has spacing. This makes me a little confused. image

This is likely due to CSS styling, check for any CSS classes or inline styles that add margin or padding to the right.

@forceddd
Copy link
Contributor Author

forceddd commented Apr 8, 2024

@cutterbl Would you please check this PR again? I submitted some new file changes.I think now this PR should be able to solve these related problems.

@cutterbl cutterbl merged commit def4934 into jquense:master Apr 15, 2024
1 check passed
@cutterbl
Copy link
Collaborator

@forceddd Another defect (#2565 ) has come up around your changes. Not only is the onSelectSlot no longer returning a slotInfo.action of 'doubleClick', but the onDoubleClickEvent handler function no longer fires either.

@forceddd
Copy link
Contributor Author

@forceddd Another defect (#2565 ) has come up around your changes. Not only is the onSelectSlot no longer returning a slotInfo.action of 'doubleClick', but the onDoubleClickEvent handler function no longer fires either.

@cutterbl Ok, today I will look at this issue and try to fix it.

@forceddd forceddd deleted the fix/issue-2536 branch May 8, 2024 03:13
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.

4 participants