@javi-er

@penyaskito, thanks for the feedback! it's fixed now.

@penyaskito, this is ready for review again.

@penyaskito, this is ready for review!

@penyaskuto, this is ready for review including Gin accent colors and dark mode options.

javi-er → created an issue.

javi-er → created an issue.

The problem with @kasey_mx patch is that it replaces the build script entirely, so any new releases of this module will have the build code reverted to the specific version of this patch without you noticing it. Ideally a config option should be added for allowing or removing linking as asked above.

@bronzehedwick yes! sorry, I should have left a comment with the test steps, you should go to the configuration at `admin/config/content/ckeditor-responsive-table` and enter a selector that will not target any tables for instance I used `table.ignore_all` which targets tables only with the `ignore_all` class which doesn't exists. So no tables should get the plugin treatment.

Thanks for the review @bronzehedwick ! there was an extra issue that I fixed, could you take another look please?

Thanks @edmoreta ! we need a functional test as well.

javi-er → created an issue.

The string for getting and setting the table selector had the wrong format, I also removed some debug code from JavaScript that was outputting some messages to the console.

This is not longer an issue, it was fixed in a previous version.

The issue that was mentioned with styles at css/responsiveTableStyles.css that are referencing a figcation element that doesn't exists should be addressed in a different issue.

There is also another issue with the Tabled plugin which is that the navigation should be set to display: none when it's not necessary since it's creating some white space between the caption and the table when the controls aren't present, this should be addressed as a new issue on the Tabled plugin.

The problem was with the Tabled plugin, see: https://github.com/Lullabot/tabled/issues/7

When the plugin was created, I didn't include the check for the `hide-caption` attribute that the CKE button adds to the `caption` element which is an indication that the user has choose to hide the caption.

javi-er → created an issue.

@bronzedwick is this a duplicate of #3484482 ?

I'm also seeing the same issue with the latest version.

Looks good to me, I checked out this branch in my local and the Tabled plugin is being initialized correctly.

This is ready for review

javi-er → created an issue.

@bronzehedwick I merged the only change that was reported by this bot, the description says that if it's kept open it will keep adding MR with changes if there are other things that can be automated for D11, I would say that it can be left open until there are a few D11 projects using this module and then we can close it for good.

This look good to me!

@bronzehedwick I think this is good to go! I can confirm that the tabled library is being loaded and the selector can be configured.
There are some styling issues though that can be fixed in a follow up ticket (at least when I test this with a project with custom styles).
Great job!

javi-er → created an issue.

@bronzehedwick it looks great! I left a bunch of comments regarding semantic aspects, once those are resolved I'll test it in my local.

@penyaskito that's a good idea, I added it as a setting, default is false in order to not disrupt existing implementations. But I'm not sure if it should be displayed or hidden by default.

This needs review, I removed the settings for hiding the toolbar, if someone still wants to have the sharebar hidden for private server, the template needs to be overridden from a custom theme. In any case these settings don't affect public server, it seems like it's a bug on the Tableau end.

javi-er → created an issue.

@rmorelli can you specify the steps to reproduce this issue, what are you seeing and what should be happening?

javi-er → made their first commit to this issue’s fork.

The Drupal 7 version is not longer supported.

The Drupal 7 version is not longer supported.

The Drupal 7 version is not longer supported.

The Drupal 7 version is not longer supported.

The Drupal 7 version is not longer supported.

The Drupal 7 version is not longer supported.

I tested this MR in my local and it's working as expected, thanks!!

Test is passing, thanks!

javi-er → made their first commit to this issue’s fork.

Thanks, merged.

I left two small comments in the PR, but I tested it locally and it's working great! existing visualizations pointing to the public domain keeps working, and if I add another domain, it works well too.

After digging through the HTML markup of the navigation and the styles, I found out that the all: revert; CSS property being applied at admin-reset-styles.pcss.css to both the draggable elements and the dropzone element is conflicting with Chrome DnD API and making it not to work, simply removing this property fixes the issue. ( related issue ✨ Reset theme css. Fixed )

However, there seems to be some other issues being caused by the navigation styles (and maybe JS?) while on the LB interface. See discussion here.

The optimal solution is to identify and separate the Navigation libraries between the core styles and everything else and while on the admin interface, and just include a library that applies the basic styling leaving out anything that's not strictly needed.

Attaching video of drag & drop working after just adding forceFallback: true to layout-builder.js.

LGTM!

We considered the implemented approach but the problem with this is that when the menu slides to the right it covers the borders for a short amount of time causing some flickering, that's why the original approach used an :after pseudoelement imo.
It's a tiny issue and not very noticeable, so I'll leave to the maintainer to decide if further action is needed, see attached video.

Here's what I found so far in case someone else is looking at this, I debugged layout builder JS implementation and found that the onEnd event of the Sortable implementation isn't being called (layout-builder.js line 165) so after some searching and testing I ended up on this Sortable issue which is reporting something similar, some of the solutions suggested adding forceFallback: true to the Sortable call, I tried this and it started working on Chrome without issues. It seems to be related to a Chrome bug but I don't have a definitive solution yet.

ckrina → credited javi-er → .

Looks good to me, both functionality and code changes.

Before and after MR videos attached.

lauriii → credited javi-er → .

javi-er → created an issue.

javi-er → created an issue.

I opened a merge request with this change, please review it.

@shiv_yadav the problem with your patch is that it will still create a <ul class="none">, the solution is actually checking that below_type is different than "none".

javi-er → created an issue.

claireristow → credited javi-er → .

claireristow → credited javi-er → .

Circling back on this, thanks!

Thanks for the support @gisle, I have contacted the project owner again and waited more than 14 days once again, I updated the issue summary to mention this.
I also plan on keep supporting the D7 version until end of life.

@gisle thank you for looking at it, I followed the steps 1-6 on the article you linked, two weeks have passed now and that's why I moved it to "Drupal.org project ownership". Since I can't see messages that I have sent to other users, I'm not 100% if I contacted him, but I just did again just in case.

Sorted.

javi-er → created an issue.

I did the suggested changes and some cleanup, however tests are still failing. I'm moving it to needs review to test the functionality, and the failing test can be addressed once SDC is part of core.

Thanks for the feedback @mherchel ! this is ready for review again.

@smustgrave the MR is green, the pipeline tab in it is empty and the CI job is building successfully: https://www.drupal.org/pift-ci-job/2687312 →

Could you specify which problems are you seeing?

@TheodorosPloumis if you do that, the CSS file will be generated inside your /pscss folder (see core/scripts/css/postcss-build.js) and it will not be loaded by SDC, so in that case core assets compilation will need to be adjusted specifically for SDC. In any case, I think it's still a mistake that it loads anything that ends with .css as if it was a stylesheet, and it will probably prevent future problems to adjust this module for taking in account these cases.

This ready for review now, I'm interested in knowing if the approach I took for splitting the styles that affects the comments wrapper and the individual comments is the right one.

There is one selector that's using duplicated styles in both places but I'm not sure if it's used on the comments form / wrapper since I can't find it inspecting the page: .add-comment__picture.

I noticed that Single Directory Components it's not only loading the compiled CSS file but also the PostCSS file, since the extension ends on .css as well, I opened an issue regarding this here: https://www.drupal.org/project/sdc/issues/3365730 ✨ SDC is loading Post CSS files, not just compiled CSS files Closed: works as designed

javi-er → created an issue.

Post CSS will compile css found in components (since it's looking for everything inside the `core` folder), will move styles and move back to needs review.