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

Upgrade to functional views #356

Merged
merged 53 commits into from
Sep 26, 2024
Merged

Conversation

JannikStreek
Copy link
Member

@JannikStreek JannikStreek commented Sep 18, 2024

Further Notes

closes #357

To be migrated

  •  landing page / static page
  • layouts
  •  admin
  • errors
  • live/brainstorming
  • live/ideas
  • Fix label display
  • fix html export
  • Fix sigil lib/mindwendel_web/templates/error/error_page.html.heex

The error view was removed so only the template is now left, see config.exs:

# Configures the endpoint
config :mindwendel, MindwendelWeb.Endpoint,
  render_errors: [
    view: MindwendelWeb.ErrorView,
    accepts: ~w(html json),
    layout: false
  ],

After Merge Checklist

  • Fix remaining issues: A label in use can't be deleted => error message isn't shown

lib/mindwendel_web.ex Outdated Show resolved Hide resolved
@JannikStreek
Copy link
Member Author

JannikStreek commented Sep 20, 2024

@PragTob if you want you can help here, as it's a lot of work. Unfortunately it wasn't so simple to just convert one view, merge it, create another PR for the next view. Views and the new functional components need different syntax so if code is shared everything explodes. So we have to do this imho as a whole. We do not need the perfect version, but throwing out views should be the goal.

My strategy: First, get everything working somehow so do not create fancy components for everything, just get it working for now with the new syntax. To do this, the functional components e.g. for forms and links have to be used. CSS can be added later. Also important, the Route helper is gone and the protected route feature has to be used (~p"/brainstormings")

Currently I am in the middle of the live/admin/brainstorming_live/edit.html.heex file. I already completed refactoring of the layout to the functional components code and as well the landing page (static page). You can have a look at my existing refactorings or also see all the links I have provided in the PR and the issue as documentation is sometimes a bit hard to find.

HTML report may remain broken, but it renders and the sockets
also work again due to me patching in the meta tage again.

Hope I didn't break anything, pls review ;)
I'm not 100% sure if taking out ideas was a mistake or if it was
meant as such so I'm not pushing forward with making more of them
visible as even more had been commented out there.

Removed more warnings, however some I was unsure about.

Left some TODO comments on places in the code where I gained
knowledge/know of work to do.
Test doesn't work yet because it tests on a whole view which
is borked yet but still an increment :)
@PragTob
Copy link
Collaborator

PragTob commented Sep 22, 2024

If we wanted to get to fixing the tests the error page is probably one of the next biggest things up, as lots of tests fail with:

** (UndefinedFunctionError) function Routes.brainstorming_show_path/3 is undefined (module Routes is not available)

which is because of the error page and so I think either this hides the real error or we need tow ork on the tests not asserting against the error page :)

@JannikStreek JannikStreek merged commit 353d253 into master Sep 26, 2024
8 checks passed
@JannikStreek JannikStreek deleted the upgrade-to-functional-views branch September 26, 2024 05:58
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.

chore: refactor to use function components
2 participants