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

PLANET-7539: Add button to convert the ToC block into a List block #2395

Closed
wants to merge 14 commits into from

Conversation

mardelnet
Copy link
Contributor

Ref: https://jira.greenpeace.org/browse/PLANET-7539


DESCRIPTION:
This PR adds a "Convert to static list" button to the Table of Content block (formerly the Submenu block).
Editors can click on it to convert the Table of Content block into a native List block and edit the block content.
This is the first step in investigating how to add WYSIWYG features to the Table of Content block.

Additionally, some JSDocs were added to make our future selves' work lighter.

@mardelnet mardelnet requested a review from a team October 3, 2024 08:30
@mardelnet mardelnet self-assigned this Oct 3, 2024
@mardelnet mardelnet requested review from sagarsdeshmukh and mleray and removed request for a team October 3, 2024 08:30
planet-4 added a commit to greenpeace/planet4-test-venus that referenced this pull request Oct 3, 2024
/unhold 9bd6d4fa-1760-4a27-8d3f-f4b2d4b9d54f
@planet-4
Copy link
Contributor

planet-4 commented Oct 3, 2024

Test instance is ready 🚀

🌑 venus | admin | blocks report | CircleCI | composer-local.json

⌚ 2024.10.07 10:29:08

@mardelnet mardelnet marked this pull request as ready for review October 3, 2024 08:58
@mleray
Copy link
Contributor

mleray commented Oct 4, 2024

Hey @mardelnet, are we meant to completely lose the CSS styles when converting to a static list? I see that currently clicking on that option makes the block look like this both in backend and frontend, but I think this block shouldn't be using our typical link styles.

Screenshot 2024-10-04 at 10 13 56

@mardelnet
Copy link
Contributor Author

Hey @mardelnet, are we meant to completely lose the CSS styles when converting to a static list? I see that currently clicking on that option makes the block look like this both in backend and frontend, but I think this block shouldn't be using our typical link styles.

Thanks, @mleray
Maybe @comzeradd can help us answer that question.
I don't know how far should we go with this PR since it's more like an investigation.

@comzeradd
Copy link
Member

are we meant to completely lose the CSS styles when converting to a static list?

Ideally we should preserve our block styling. I see that after the conversion it's just a list with a .wp-block-list class. Maybe there is a way to add an extra css class on conversion to overwrite styles?

For the scope of this PR/ticket, we would just need to check that it's feasible. Then we can check with the design team if they are fine with the "Convert to static" workflow.

planet-4 added a commit to greenpeace/planet4-test-venus that referenced this pull request Oct 7, 2024
/unhold bc835bae-e1df-40e2-8f58-93148bbbeb24
@mardelnet mardelnet changed the base branch from main to feature/add-border-radius-to-theme October 7, 2024 10:16
@mardelnet mardelnet changed the base branch from feature/add-border-radius-to-theme to main October 7, 2024 10:16
planet-4 added a commit to greenpeace/planet4-test-venus that referenced this pull request Oct 7, 2024
/unhold efec7eee-56c0-4f8d-87f8-5f2cbba97184
@mardelnet mardelnet force-pushed the PLANET-7539_submenu-block-wysiwyg branch from 387c3b7 to 17cbc64 Compare October 7, 2024 10:19
planet-4 added a commit to greenpeace/planet4-test-venus that referenced this pull request Oct 7, 2024
/unhold c51304f6-6c45-4ba1-9c52-4c1c770f12cf
@mardelnet mardelnet marked this pull request as draft October 7, 2024 10:44
@mardelnet
Copy link
Contributor Author

mardelnet commented Oct 7, 2024

For the scope of this PR/ticket, we would just need to check that it's feasible. Then we can check with the design team if they are fine with the "Convert to static" workflow.

@mleray @comzeradd
It is feasible to keep the styles and you can check it with the last changes introduced to this PR (check the attached image, too).
The only visual element not yet migrated is the multiple-column arrangement.
This is not going to be very easy to mimic, since the native List block is not ready for using more than one column.
Please, let me know if you want me to keep working on introducing the multi-columns feature, or not, if that's out of this PR scope.

About-us-Greenpeace

@mardelnet mardelnet marked this pull request as ready for review October 7, 2024 11:00
@mleray
Copy link
Contributor

mleray commented Oct 8, 2024

@mardelnet I personally think it looks good enough like this, except maybe the fact that it's full width!

@comzeradd
Copy link
Member

This is not going to be very easy to mimic, since the native List block is not ready for using more than one column.
Please, let me know if you want me to keep working on introducing the multi-columns feature, or not, if that's out of this PR scope.

Maybe there is a way to have a flex wrapper for that list to be able to use something like column-count. But let's put this on hold till we consult with the design team to see if the overall solution is what we want to do.

@mardelnet
Copy link
Contributor Author

Closing this PR since the secondary navigation will be a different block.

@mardelnet mardelnet closed this Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants