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

Update to liveview 1.0 #687

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

thomasfortes
Copy link
Contributor

The first commit closes #404 and the following one format the code to use the new template interpolation with brackets.

@thomasfortes
Copy link
Contributor Author

thomasfortes commented Dec 4, 2024

Still need to wait for sentry to be updated so I can remove the the phoenix_live_view override in the demo app, outside of that it should be ok.

@thomasfortes thomasfortes marked this pull request as ready for review December 4, 2024 20:38
@pehbehbeh
Copy link
Member

Thanks! <3 I will take a look tomorrow.

We will probably do the 0.9 release without this and a 0.10 release with LiveView 1.0 support shortly after.

@katafrakt
Copy link

FYI Sentry has been updated to use LiveView 1.0 . Would love to see Backpex on LiveView 1.0 ;)

@@ -427,7 +427,11 @@ defmodule Backpex.Fields.Upload do
upload_key = assigns.field_options.upload_key
uploads_allowed = not is_nil(assigns.field_uploads)
translate_error_fun = Map.get(assigns.field_options, :translate_error, &Function.identity/1)
form_errors = BackpexForm.translate_form_errors(assigns.form[assigns.name], translate_error_fun)

errors =
Copy link
Collaborator

Choose a reason for hiding this comment

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

@thomasfortes I guess we have the same error also in the Upload field. I put avatar in the required_fields list in user.ex and it immediately shows that an upload is required even though I did not use the upload.

We also have this issue in the multi select. You can see it in the “Send email” action modal (users table), but we had this multi select issue before your LiveView 1.0 update so we could fix this in another MR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm not sure if I'm fully satisfied with the solution in the commit so let me know what you think, the upload changes are put into the changes by a user provided function, to avoid breaking user code I had to create a hidden field to keep track if the field has been manipulated and do some params manipulation in the put_upload_change function in the form_component.ex, the variables are terribly named and could use some kindness, it also probably can be cleaned a bit, but as it is it should "work".

There's a caveat, if you insert an image and then remove it it will not trigger the error until you interact with another field, which isn't perfect but it is better than the situation we have right now where it triggers when you interact with any field, but even this caveat should be solvable fixing the cancel-entry event.

Happy holidays :)

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.

Remove phx-feedback-for
4 participants