Skip to content

fix: context info for event actions being null#141

Open
rajabilal555 wants to merge 1 commit intoGuavaCZ:mainfrom
rajabilal555:main
Open

fix: context info for event actions being null#141
rajabilal555 wants to merge 1 commit intoGuavaCZ:mainfrom
rajabilal555:main

Conversation

@rajabilal555
Copy link
Copy Markdown

fixes: #140

@paulhennell
Copy link
Copy Markdown
Contributor

Can confirm this fixes the issue - it needs to be accessible for livewire.
@lukas-frey can we get this merged?

@lukas-frey
Copy link
Copy Markdown
Contributor

Thanks for the PR!

I'm not sure I like allowing the raw data to be accessed though. Can you please refactor the DateSelectInfo and other related value objects and include the CalendarResource in these objects instead in order to align with the current API to access the properties?

That way you should be able to access the resource using the intended API.

@paulhennell
Copy link
Copy Markdown
Contributor

I was looking at it earlier because the protected to public fix felt instinctively off, so wondered if there was another way. But the code doesn't actually access the property anywhere directly, it's only when livewire is re-hydrating the component and filling it from the front end values. With it protected the values get lost between requests, breaking all the context menu functions. I don't understand the lifecycle enough to see how to avoid it storing the data in the front end.

Possibly it could have the [#[Locked]] attribute to imply it's public only because of livewire reasons, but all the data is from the front end anyway and it doesn't do anything to stop back end changes so I don't know if it really improves the situation any.

@lukas-frey
Copy link
Copy Markdown
Contributor

Hi, thanks @paulhennell that could work.

However I can still not replicate any of the issues, can anyone please create a small repository and replicate the issue there, so I can look into it in more detail? Thanks.

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.

[Bug]: getDateSelectContextMenuActions not working

3 participants