🇵🇹Portugal @bbruno

Up for review! Would be great if someone can take a look :)

Fixed MR merge conflicts

@sirclickalot Thanks for the extra context and quick reply! I just took a look and yes, when inserting a paragraph within a paragraph this uses '.button--small' and it happens because it comes from the Gin template.

I was only checking from my side when adding the 'first level' paragraphs, which use a hardcoded class added at FrontendEditingController.php.

Could you confirm if the MR I provided above fixes this issue?

This has been fixed, the SVG did indeed contain a built in shadow, it was also not correctly sized and included an empty space around the image itself which made it not possible to add a shadow from the CSS side - this was also fixed. The shadow is now applied using CSS and can be overwritten with CSS up to the user's liking

Up for review

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to hidden.

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to active.

bbruno → changed the visibility of the branch 3441696-remove-svg-shadow to hidden.

I can replicate this, working on it

bbruno → made their first commit to this issue’s fork.

Hello @sirclickalot, you are right in saying that this CSS you are encountering does not seem to have a use. More even so since from previous commits we are no longer using the '.button--small' class on this paragraph list, but '.button--large'.

I have provided an MR that completely removes this CSS, as I mentioned does not have a use. However, I am still intrigued as to why your environment still contains these small buttons, considering that the class was changed in the same merge request as the CSS above.

I invite you to take a look and suggest any other options if this is still not as expected. Thank you!

bbruno → made their first commit to this issue’s fork.

bbruno → made their first commit to this issue’s fork.

We have worked on this at 1xINTERNET and would want to share our work

We have already implemented this at 1xINTERNET and would want to contribute our work

bbruno → created an issue.

bbruno → created an issue.

Added a proposed solution to the issue as mentioned above we have worked on this at 1xINTERNET and would contribute our work

It seems that this happened when the ai_ckeditor module was initially ported over, would be nice if someone can take a look here. Thanks!

bbruno → created an issue.

This is a nice idea, I have tested it locally and it works as expected. I agree with mrdalesmith that it perhaps is a little non-standard, but in my opinion provides a much better ux than the current option.

Non-related to the issue itself, but the changes in the package-lock.json and yarn.lock indicate that we have multiple package handlers at work here, it would be best to stick to one to avoid any inconsistencies due to unsynced lock files.

bbruno → created an issue.

Created the MR above which contains a check for if the body element does not contain the class .frontend-editing--hidden before applying the related styling, as is the case for the rest of the hidden styles. Up for review :)

bbruno → created an issue.

Hi sirclickalot, the SVGs that are used in the actions links are actually background images applied to pseudo-element '::before' on each link.

Since these are black, they can be easily manipulated using a 'Filter' style.

For example, this filter when targeting this '::before', will turn the icons red:

.frontend-editing__action::before { 
 filter: invert(20%) sepia(90%) saturate(4516%) hue-rotate(357deg) brightness(97%) contrast(135%);
}

You can search online for 'CSS filter black to hex' and find tools that will provide you with the correct filter styling to match your desired color

Added the changes into the MR above.

This removes the 'bottom' positioning and instead replaces it with the 'fixed' positioning, which is now applied whenever the element's top goes out of view. It also means we can get rid of the 'bottom' observer, since there is no longer a change in positioning depending on if the bottom of an element becomes visible.

Up for review.

This adjustments make sense, I will start taking a look here.

breidert → credited bbruno → .

MR updated, please proceed Artem.

Updated MR !73 to use the IntersectionObserver API as an alternative to the Scroll event listener due to performance.

Back to Needs Review

Rerolling the last patch as it is not working with the latest recommended version. It had been very useful as the photoswipe images do not come by default with any lazy-loading attributes and there are no settings to do so.

Thanks SirClickalot for that last comment, it helped with reaching a possible solution since we can not replicate your issue. Updated the MR with a fix that should hopefully sort this out. Up for review.

The new custom color variable has been implemented on the toggle. Up for review.

Bug where the hover mode padding was still being applied although the toggle is set to 'Off' has been fixed. Putting up for review.

bbruno → changed the visibility of the branch 3439916-introduce-color-customization to active.

bbruno → changed the visibility of the branch 3439916-introduce-color-customization to hidden.

Added an MR which reduces the size of the 'Add' links to only the button. This change only applies when not using the Hover Highlight Mode - for those the blue area will still be clickable.

bbruno → created an issue.

Removed the what seems to be unnecessary margin: 0; styling for nested elements which could be causing 'jiggling' if the element already had an existing margin applied to it.

bbruno → created an issue.

bbruno → created an issue.

@DieterHolvoet Apologies I should've been more clear. I could not reproduce the problems in #17 and above with this new patch, based on the previously suggested one. Up for review to see if it fixes it for the rest.

Could not reproduce any of the problems above, the correct option is selected as well as the UI being correctly updated.

I re-rolled the last patch to apply to the latest dev. Please review.