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

Page Builder - enable the editor on New pages #172

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

ChristianTovar
Copy link
Collaborator

@ChristianTovar ChristianTovar commented Jun 13, 2024

Demo

demo.4.34.50.PM.mov

@ChristianTovar ChristianTovar force-pushed the 131-page-builder-enable-editor-on-new-pages branch from 9bf4d97 to afcdb7c Compare June 14, 2024 21:20
@ChristianTovar ChristianTovar self-assigned this Jun 14, 2024
@@ -34,11 +34,15 @@ defmodule Beacon.LiveAdmin.PageEditorLive.New do
end

defp build_new_page(site, [layout | _] = _layouts) do
id = Ecto.UUID.generate()
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of pre populating dummy data here that will be rendered in the form, what if we populate the data (only if it's empty) on the actual call to build the page ast? (the page ast is used on the visual editor only)

%{data: builder_page} <- WebAPI.Page.show(page.site, page) do

page is resolved from the changeset so if the required fields are present (id, title, path) then it should work, otherwise we could fill with dummy data to make the visual editor work. This way user won't see the dummy data on the form.

Wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I've been trying to see what changes with your suggestion. I think populating just to make it render is fine, however populating at the LiveView level is also important because it not only helps rendering the visual editor but it also provided an appropriate state to the assigns so that the page can be drafted, saved and also published.

Copy link
Collaborator Author

@ChristianTovar ChristianTovar Jul 8, 2024

Choose a reason for hiding this comment

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

@leandrocp Please take a look to the updated demo, drag and drop functionality has now been enabled and feature is fully functional!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think populating just to make it render is fine

We can go with that for this version 👍🏻

mix.lock Outdated Show resolved Hide resolved
@ChristianTovar ChristianTovar force-pushed the 131-page-builder-enable-editor-on-new-pages branch from 31a5f3c to ca86272 Compare June 26, 2024 20:25
@ChristianTovar ChristianTovar force-pushed the 131-page-builder-enable-editor-on-new-pages branch from 2144401 to ab3feda Compare July 5, 2024 17:19
@ChristianTovar ChristianTovar marked this pull request as ready for review July 8, 2024 17:30
Copy link
Contributor

@leandrocp leandrocp left a comment

Choose a reason for hiding this comment

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

Hey @ChristianTovar that looks good. What about changing the auto generated path with timestamp instead? (like you mentioned yesterday)

lib/beacon/live_admin/live/page_editor_live/new.ex Outdated Show resolved Hide resolved
@@ -34,11 +34,15 @@ defmodule Beacon.LiveAdmin.PageEditorLive.New do
end

defp build_new_page(site, [layout | _] = _layouts) do
id = Ecto.UUID.generate()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think populating just to make it render is fine

We can go with that for this version 👍🏻

@ChristianTovar ChristianTovar force-pushed the 131-page-builder-enable-editor-on-new-pages branch from 55d1dde to 8aeb0f7 Compare July 12, 2024 16:02
@ChristianTovar ChristianTovar force-pushed the 131-page-builder-enable-editor-on-new-pages branch from c27e7a4 to 6df0229 Compare July 12, 2024 16:20
Copy link
Contributor

@leandrocp leandrocp left a comment

Choose a reason for hiding this comment

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

🎉

@ChristianTovar ChristianTovar merged commit 7b55020 into main Jul 12, 2024
3 checks passed
@ChristianTovar ChristianTovar deleted the 131-page-builder-enable-editor-on-new-pages branch July 12, 2024 16:39
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.

None yet

2 participants