[bug 1200762] Logo swap template blockable and clickable.

[glogiotatidis]

Based on logo swap template and the simple snippet with close https://github.com/mozilla/snippets/blob/master/templates/simple-snippet.html

Posted on stage: https://snippets.allizom.org/admin/base/snippettemplate/43/

Note that the close functionality cannot be tested in preview mode.

See also 1172579

[glogiotatidis]

@schalkneethling r?

[schalkneethling]

I would prefer if we use px here as apposed to points

[schalkneethling]

So, after finally getting this up and running locally, thanks @glogiotatidis This works as expected. Shows the snippet, replaced the Fx logo with whatever image you set as the replaced image. Hovering over the snippet presents a close icon, and clicking on this removes the snippet.

[schalkneethling]

Does most/all snippets share these rules?

.block-snippet-overlay-{{snippet-id}} {
    position: absolute;
    height: 100%;
    width: 100%;
    left: 0px;
    top: 0px;
    padding-top: 5px;
    padding-bottom: 5px;
    z-index: -10;
}

If that is the case, we can probably get a lot of reuse and thus less CSS over the wire, if we have a standard class that applies to all and not have it included for each snippet. Perhaps the way snippets are loaded and such prevents us though, but thought I would throw it out there.

[schalkneethling]

Not sure where the glow class lives, but I believe (looking at the tiles hover state) that the rules needs to be as follows:

.block-snippet-overlay-glow-{{snippet-id}} {
   visibility: visible;
   border: 2px solid fff;
   box-shadow: 0 0 6px 2px #4CB1FF;
}

[schalkneethling]

Can we change this to an a element or, preferably a button element (will need some additional styling to remove the border, background etc.) and then add a title attribute with the value Remove this or similar.

[schalkneethling]

The above is partly related to this comment in bugzilla https://bugzilla.mozilla.org/show_bug.cgi?id=1172579#c26 but, also for a11y. Thanks!

[glogiotatidis]

Can we change this to an a element or, preferably a button element (will need some additional styling to remove the border, background etc.) and then add a title attribute with the value Remove this or similar.

done.

The above is partly related to this comment in bugzilla https://bugzilla.mozilla.org/show_bug.cgi?id=1172579#c26 but, also for a11y. Thanks!

Great point again. I want to tackle l10n as well so for the first iteration let's ship this without any text. I updated the bug.

[schalkneethling]

You can change the above to:

.block-snippet-overlay-glow-{{snippet-id}} {
   visibility: visible;
   border: 2px solid fff;
   box-shadow: 0 0 6px 2px #4CB1FF;
}

and it will match tiles exactly.

[glogiotatidis]

If that is the case, we can probably get a lot of reuse and thus less CSS over the wire, if we have a standard class that applies to all and not have it included for each snippet. Perhaps the way snippets are loaded and such prevents us though, but thought I would throw it out there.

Great point. The close button is a new feature so I want to create a couple of templates to see how much is re-usable between them. Then we can share classes between snippets. That includes the close buttons as well.

[glogiotatidis]

This is updated with the review comments. Multiple commits for your convenience @schalkneethling, I'll squash before merging.

[schalkneethling]

This is all good. r+ 🐣

[glogiotatidis]

Thanks!