aesmail / kaffy

Powerfully simple admin package for phoenix applications
https://kaffy.fly.dev/admin/
MIT License
1.3k stars 153 forks source link

[Feature] Phoenix HTML 4.0 not supported #305

Closed nathany-copia closed 1 month ago

nathany-copia commented 6 months ago

Versions Used Kaffy: 0.10.2 Phoenix: 1.7.10 Elixir: 1.16.0

What's actually happening?

==> kaffy
Compiling 26 files (.ex)

== Compilation error in file lib/kaffy/resource_error.ex ==
** (RuntimeError) use Phoenix.HTML is no longer supported in v4.0.

To keep compatibility with previous versions, add {:phoenix_html_helpers, "~> 1.0"} to your mix.exs deps
and then, instead of "use Phoenix.HTML", you might:

    import Phoenix.HTML
    import Phoenix.HTML.Form
    use PhoenixHTMLHelpers

What should happen instead?

Update for Phoenix HTML 4.0 https://hexdocs.pm/phoenix_html/changelog.html

Screenshots If applicable, add screenshots to help explain your problem.

drobban commented 5 months ago

Well. Not sure if this is to classify as a bug.

Im running Kaffy: 0.10.2 Phoenix: 1.7.10 Phoenix_live_view: 0.20.3 Phoenix_html: 3.3 Elixir: 1.16.0

and I have not encountered the runtime error you have - Trying to upgrade to Phoenix_html: 4.0 is stopped - as kaffy depend on Phoenix_html: 3.x

Bumping the version of Phoenix_html: 3.x to 4.x in kaffy - I guess the solution then is to do what the changelog and runtime error suggest. Change the use of Phoenix.HTML to import...... and add the dependency phoenix_html_helpers, "~> 1.0"

import Phoenix.HTML
import Phoenix.HTML.Form
use PhoenixHTMLHelpers

I dont have any plans to bump up to phoenix_html 4.0 in any current project for the moment. Perhaps this would be a perfect moment for you to test making the change in kaffy repo - bumping the versions and make a pull request?

Otherwise, test to downgrade Phoenix_html and follow the instructions in the readme.

Happy hunting.

nathany-copia commented 5 months ago

We can just continue to use Phoenix HTML 3.3 for now.

I updated the title to classify it as a feature request, but I can't modify the label myself.

drobban commented 5 months ago

@nathany-copia I made a pull-request solving the issue you reported!

Would be awesome if you could test it out and report back if you find any bugs/errors. Pull request #306

To test run it you can change your dep in mix.exs to use {:kaffy, git: "https://github.com/drobban/kaffy.git", branch: "phx_1.7.10_breaking"},

update If you want to test it out use this line instead and reference a specific commit instead {:kaffy, github: "drobban/kaffy", ref: "fd0ee0b"}

nathany-copia commented 5 months ago

Thank you @drobban.

My server and tests are all working after updating a few spots from use Phoenix.HTML to the following:

import Phoenix.HTML
import Phoenix.HTML.Form
use PhoenixHTMLHelpers

Though I must admit, I'm not using Kaffy very heavily, but what I saw continued to work.

I used the line {:kaffy, github: "drobban/kaffy", ref: "fd0ee0b"} in mix.exs because I like to lock down the commit being pulled in.

drobban commented 5 months ago

@nathany-copia Smart move 😅 didn't really think so much about until now after you mentioned it. Pinning to commit is definitely safer and less risk of headaches.

phortx commented 4 months ago

Hi! Any plans to merge and release that change? :)

drobban commented 4 months ago

Hi! Any plans to merge and release that change? :)

Would guess that @aesmail will have a look at it when he has time over for it. At mean time, it would be great if you could test that change out and report back - Super simple to do, just specify the commit in your deps list. =)

phortx commented 4 months ago

@drobban thanks for the answer :) I plan to test that branch in the next week and give you feedback :)

drobban commented 4 months ago

@phortx as @nathany-copia pointed out

"I used the line {:kaffy, github: "drobban/kaffy", ref: "fd0ee0b"} in mix.exs because I like to lock down the commit being pulled in."

Is a much smarter move then to use branch as I did in my example... In that way you wont be surprissed by any changes in that branch that I might make.

nathany-copia commented 4 months ago

It's probably best to make a fork and use that. Otherwise the branch (and commit) could be deleted (such as when merged) and break the build.

antonioparisi commented 3 months ago

is this repo still maintained?

drobban commented 3 months ago

is this repo still maintained?

it is as far as i know. Why do you ask?

estromenko commented 2 months ago

Any updates?

drobban commented 2 months ago

Any updates?

I guess if there where any to report, you would of seen it in the relevant issue topic.

Feel free to read this until there is any updates. https://github.com/sponsors/aesmail

You can also test the pull request solving this issue by starting using it and report back if everything works as intended.

Instructions is found in the thread.

ghenry commented 2 months ago

We've merged a fix for this. Please re-test.

murraybryant commented 2 months ago

works for me now

nathany-copia commented 1 month ago

Thank you.

I'm using {:kaffy, github: "aesmail/kaffy", ref: "d864ea1"} which is working here.