Claro: Remove svg from indentation.html.twig file

Hi @Gauravvv, I have Reviewed the patch on drupal 10.1.x.The patch was successfully applied. Need RTBC+1

Verified the patch #3 on Drupal version 10.1.x.
The patch works fine and I have added the before and after screenshot for reference.

This needs screenshots of use cases where indentation.html.twig is in use. Tabledrag, including situations where the tree lines are visible would work.

This does not (and never did) need screenshots of a patch applying or source code. The patches + testbot do that for us already.

Doesn't seem to cause any disruption.

@Gauravvvv Thanks for the work. Applied patch #3 cleanly on Drupal version 10.1.x. Please see attached screenshot.

Random failure.

This needs a conversion to an MR now, shame it's been stuck on random test failures for so long.

Hello,

I have converted the patch into MR. Please review.

Thank you!

restoring previous status

I'm not sure about this change. I think the reason this is inlined is due to core/themes/claro/images/icons/002e9a/plus.svg core/themes/claro/images/icons/545560/plus.svg etc... existing. If you need to have a plus icon in Claro then, I think, you should be using those. The svg in indentation.html.twig is for a very specific use-case - hence the inlining.

However, maybe the Claro maintainers will think differently, will ping them. However, if I'm right then perhaps this issue should be about adding a comment to Claro's indentation.html.twig about why the svg is inlined.

@alexpott is correct, this is a very specialized use case. It defaults to being a blank square, and there are several hidden svgs that use tabbledrag specific functionality to become visible on mousedown to display lines that visually connect related drag items. (so very coupled to tabledrag JS)

Keeping it inline makes it easier to maintain and allows for improvements to the Tabledrag UI without having to worry about BC with other uses of the image. I don't think it is worth moving it to a separate image unless there are specific use cases to justify it. The reported use cases seem more hypothetical.

If there is something more specific that seems best addressed by un-inlining the SVG, comment or update the IS with it an reopen, but as reported this seems like something best left alone.

In the Stable theme, we're using space for indentation in core/themes/stable9/templates/admin/indentation.html.twig. We have different behaviors within the core themes. Can we make them similar for both? We can make changes to Claro or Stable.

This is unlikely

The Stable theme, as the name suggests, exists to provide a stable set of markup and styling that will not make any changes that could potentially result in regressions (sometimes referred to as the "stable fence"). Updating Stable to work like Claro would be counter to this.

Claro is arguably a better implementation and would not want to roll it back to a pre-2019 implementation for the sake of making the approach similar in two themes - particularly since Stable's purpose is to provide a minimal frozen-in-amber theme intended for use as a base theme, while Claro is built to be an admin theme that accommodates innovations across updates.

Simlar to my stance in #25, if there's a non-theoretical real-world use case that this might address, making the change is a conversation worth having. If the desire is to have consistent approaches across themes without that real-world use case, it would not get FEFM signoff from me.

f the desire is to have consistent approaches across themes without that real-world use case, it would not get FEFM signoff from me.

+1

Themes can have different approaches to CSS / HTML and design - that's kinda the point.

Should this be closed out?

Going to close out.