Revert "[4.0] Webauth inline SVG button with fill" by infograf768 · Pull Request #31516 · joomla/joomla-cms

infograf768 · 2020-11-28T10:15:13Z

This reverts commit 31d2bdf.

infograf768 · 2020-11-28T10:20:04Z

After discussion in maintainers chat, we decided to revert this PR until we have a full API in 4.1 or later.
Until then you are free to use a similar trick I used for the alerts (via css) or a direct file_get_contents

@sandewt
Same for your vote plugin, You will have to modify it to use what you had already done before this merge.

Thanks!

N6REJ · 2020-11-28T20:04:14Z

@infograf768 would you link the alerts work please.

C-Lodder · 2020-11-29T07:33:04Z

I really don't understand why the project can't simply use an inline SVG, like so:

<button>Web Authentication <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="1.5em"><path fill="currentColor" d="M15.287 3.63a8.407 8.407 0 00-8.051 7.593h.55a7.805 7.805 0 012.24-4.713 5.825 5.825 0 00.924.695c-.608 1.177-.98 2.556-1.082 4.018h.135c.105-1.467.485-2.819 1.065-3.947.745.434 1.623.754 2.577.94a27.83 27.83 0 00-.25 3.763h-.847v.135h.847c.003 1.334.09 2.617.25 3.764-.954.185-1.832.506-2.577.94a9.997 9.997 0 01-.978-3.137h-.137c.164 1.16.502 2.25.997 3.208a5.825 5.825 0 00-.924.695 7.805 7.805 0 01-2.255-4.875H7.22A8.407 8.407 0 0024 12.034a8.398 8.398 0 00-.688-3.333 8.407 8.407 0 00-8.025-5.072zm.315.546c.155 0 .31.005.464.014.365.34.708 1.07.983 2.114a16.518 16.518 0 01.357 1.79 10.173 10.173 0 01-1.804.16 10.173 10.173 0 01-1.805-.16 16.519 16.519 0 01.357-1.79c.275-1.045.618-1.775.983-2.114a7.97 7.97 0 01.465-.014zm-.665.028c-.345.392-.658 1.093-.913 2.065a16.639 16.639 0 00-.36 1.8c-.939-.183-1.802-.498-2.533-.926.686-1.283 1.635-2.264 2.73-2.775a7.874 7.874 0 011.076-.164zm1.33 0a7.856 7.856 0 011.084.168c1.092.513 2.037 1.492 2.721 2.771-.73.428-1.594.743-2.533.927a16.64 16.64 0 00-.36-1.8c-.255-.972-.568-1.673-.912-2.066zm-2.972.314c-.655.407-1.257.989-1.776 1.73a8.166 8.166 0 00-.506.825 5.69 5.69 0 01-.891-.67 7.814 7.814 0 013.173-1.885zm4.624.006a7.862 7.862 0 013.164 1.877 5.692 5.692 0 01-.893.672 8.166 8.166 0 00-.506-.825c-.516-.738-1.115-1.318-1.765-1.724zm3.26 1.985a7.858 7.858 0 011.638 2.419 7.802 7.802 0 01.642 3.051h-2.095c-.01-1.74-.398-3.396-1.11-4.774a5.823 5.823 0 00.925-.696zm-1.044.767c.679 1.32 1.084 2.945 1.094 4.703h-3.42a27.863 27.863 0 00-.251-3.763c.954-.186 1.833-.506 2.577-.94zm-6.357.965a10.299 10.299 0 001.824.16 10.299 10.299 0 001.823-.16c.16 1.138.246 2.413.249 3.738h-1.178a1.03 1.03 0 01-.093.135h1.27a27.71 27.71 0 01-.248 3.739 10.397 10.397 0 00-3.647 0 27.733 27.733 0 01-.248-3.739h1.294a.99.99 0 01-.09-.135H13.53c.003-1.325.088-2.6.248-3.738zM2.558 9.37a2.585 2.585 0 00-2.547 2.35c-.142 1.541 1.064 2.842 2.566 2.842 1.26 0 2.312-.917 2.533-2.124h4.44v.972h.946v-.972h.837v1.431h.945v-2.376H5.11A2.586 2.586 0 002.558 9.37zm-.058.965a1.639 1.639 0 011.707 1.637 1.64 1.64 0 01-1.639 1.638 1.639 1.639 0 01-.068-3.275zm13.09.388a.75.75 0 00-.345 1.404l-.383 1.958h1.5l-.383-1.958a.75.75 0 00.384-.654.75.75 0 00-.773-.75zm2.218 1.391h3.421c-.01 1.758-.415 3.384-1.094 4.704-.744-.434-1.623-.755-2.577-.94a27.81 27.81 0 00.25-3.764zm3.556 0h2.095a7.805 7.805 0 01-2.281 5.47 5.825 5.825 0 00-.924-.696c.712-1.378 1.1-3.033 1.11-4.774zm-5.52 3.703a10.284 10.284 0 011.562.156 16.518 16.518 0 01-.357 1.791c-.275 1.045-.618 1.774-.982 2.114a7.972 7.972 0 01-.93 0c-.365-.34-.708-1.07-.983-2.114a16.519 16.519 0 01-.357-1.79 10.284 10.284 0 012.048-.157zm1.695.181c.94.184 1.803.5 2.533.926-.686 1.284-1.635 2.265-2.73 2.776a7.874 7.874 0 01-1.075.164c.344-.393.657-1.094.913-2.065a16.64 16.64 0 00.359-1.8zm-3.874 0a16.648 16.648 0 00.359 1.8c.255.973.568 1.674.913 2.066a7.873 7.873 0 01-1.075-.164c-1.096-.511-2.045-1.492-2.731-2.775.73-.428 1.594-.743 2.534-.927zm-2.652.997a8.16 8.16 0 00.506.825c.52.741 1.121 1.323 1.776 1.73a7.814 7.814 0 01-3.174-1.884 5.694 5.694 0 01.892-.67zm9.178 0a5.694 5.694 0 01.891.67 7.814 7.814 0 01-3.173 1.885c.654-.407 1.256-.989 1.775-1.73a8.16 8.16 0 00.507-.825z"></path></svg></buttton>

It will solve all the problems you've been having with it.

No additional CSS required
No API required
No alternative colour required. It will match that of the button text colour
The size is 1.5x relative to the button text

infograf768 · 2020-11-29T07:35:04Z

@N6REJ
#30516

infograf768 · 2020-11-29T08:54:12Z

@C-Lodder @N6REJ
In fact it indeeed could be done by taking off the part concerning api and using

'svg' => '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="2.5em"><path fill="currentColor" d="M15.287 3.63a8.407 8.407 0 00-8.051 7.593h.55a7.805 7.805 0 012.24-4.713 5.825 5.825 0 00.924.695c-.608 1.177-.98 2.556-1.082 4.018h.135c.105-1.467.485-2.819 1.065-3.947.745.434 1.623.754 2.577.94a27.83 27.83 0 00-.25 3.763h-.847v.135h.847c.003 1.334.09 2.617.25 3.764-.954.185-1.832.506-2.577.94a9.997 9.997 0 01-.978-3.137h-.137c.164 1.16.502 2.25.997 3.208a5.825 5.825 0 00-.924.695 7.805 7.805 0 01-2.255-4.875H7.22A8.407 8.407 0 0024 12.034a8.398 8.398 0 00-.688-3.333 8.407 8.407 0 00-8.025-5.072zm.315.546c.155 0 .31.005.464.014.365.34.708 1.07.983 2.114a16.518 16.518 0 01.357 1.79 10.173 10.173 0 01-1.804.16 10.173 10.173 0 01-1.805-.16 16.519 16.519 0 01.357-1.79c.275-1.045.618-1.775.983-2.114a7.97 7.97 0 01.465-.014zm-.665.028c-.345.392-.658 1.093-.913 2.065a16.639 16.639 0 00-.36 1.8c-.939-.183-1.802-.498-2.533-.926.686-1.283 1.635-2.264 2.73-2.775a7.874 7.874 0 011.076-.164zm1.33 0a7.856 7.856 0 011.084.168c1.092.513 2.037 1.492 2.721 2.771-.73.428-1.594.743-2.533.927a16.64 16.64 0 00-.36-1.8c-.255-.972-.568-1.673-.912-2.066zm-2.972.314c-.655.407-1.257.989-1.776 1.73a8.166 8.166 0 00-.506.825 5.69 5.69 0 01-.891-.67 7.814 7.814 0 013.173-1.885zm4.624.006a7.862 7.862 0 013.164 1.877 5.692 5.692 0 01-.893.672 8.166 8.166 0 00-.506-.825c-.516-.738-1.115-1.318-1.765-1.724zm3.26 1.985a7.858 7.858 0 011.638 2.419 7.802 7.802 0 01.642 3.051h-2.095c-.01-1.74-.398-3.396-1.11-4.774a5.823 5.823 0 00.925-.696zm-1.044.767c.679 1.32 1.084 2.945 1.094 4.703h-3.42a27.863 27.863 0 00-.251-3.763c.954-.186 1.833-.506 2.577-.94zm-6.357.965a10.299 10.299 0 001.824.16 10.299 10.299 0 001.823-.16c.16 1.138.246 2.413.249 3.738h-1.178a1.03 1.03 0 01-.093.135h1.27a27.71 27.71 0 01-.248 3.739 10.397 10.397 0 00-3.647 0 27.733 27.733 0 01-.248-3.739h1.294a.99.99 0 01-.09-.135H13.53c.003-1.325.088-2.6.248-3.738zM2.558 9.37a2.585 2.585 0 00-2.547 2.35c-.142 1.541 1.064 2.842 2.566 2.842 1.26 0 2.312-.917 2.533-2.124h4.44v.972h.946v-.972h.837v1.431h.945v-2.376H5.11A2.586 2.586 0 002.558 9.37zm-.058.965a1.639 1.639 0 011.707 1.637 1.64 1.64 0 01-1.639 1.638 1.639 1.639 0 01-.068-3.275zm13.09.388a.75.75 0 00-.345 1.404l-.383 1.958h1.5l-.383-1.958a.75.75 0 00.384-.654.75.75 0 00-.773-.75zm2.218 1.391h3.421c-.01 1.758-.415 3.384-1.094 4.704-.744-.434-1.623-.755-2.577-.94a27.81 27.81 0 00.25-3.764zm3.556 0h2.095a7.805 7.805 0 01-2.281 5.47 5.825 5.825 0 00-.924-.696c.712-1.378 1.1-3.033 1.11-4.774zm-5.52 3.703a10.284 10.284 0 011.562.156 16.518 16.518 0 01-.357 1.791c-.275 1.045-.618 1.774-.982 2.114a7.972 7.972 0 01-.93 0c-.365-.34-.708-1.07-.983-2.114a16.519 16.519 0 01-.357-1.79 10.284 10.284 0 012.048-.157zm1.695.181c.94.184 1.803.5 2.533.926-.686 1.284-1.635 2.265-2.73 2.776a7.874 7.874 0 01-1.075.164c.344-.393.657-1.094.913-2.065a16.64 16.64 0 00.359-1.8zm-3.874 0a16.648 16.648 0 00.359 1.8c.255.973.568 1.674.913 2.066a7.873 7.873 0 01-1.075-.164c-1.096-.511-2.045-1.492-2.731-2.775.73-.428 1.594-.743 2.534-.927zm-2.652.997a8.16 8.16 0 00.506.825c.52.741 1.121 1.323 1.776 1.73a7.814 7.814 0 01-3.174-1.884 5.694 5.694 0 01.892-.67zm9.178 0a5.694 5.694 0 01.891.67 7.814 7.814 0 01-3.173 1.885c.654-.407 1.256-.989 1.775-1.73a8.16 8.16 0 00.507-.825z"></path></svg>',

instead of

joomla-cms/plugins/system/webauthn/src/PluginTraits/AdditionalLoginButtons.php

Line 156 in 275a5f2

    
           'svg'                => HTMLHelper::_('icons.svg', 'plg_system_webauthn/webauthn.svg', true),

in AdditionalLoginButtons.php

Note: 2.5em instead of 1.5em tobe as near as possible from the similar buttons size

In this case we could close this PR and create a new one where we take off the api aspect and the css fill color, but keep the css concerning margin-right and left depending on ltr/rtl
.svg files in build would be deleted, otherwise the rest of the original pr would be kept.

@HLeithner
what do you think?

sandewt · 2020-11-29T09:31:20Z

In fact it indeeed could be done by taking off the part concerning api and using

The disadvantage I think is that this code is difficult to read and maintain.
That's why I prefer to load a .svg file. As a separate building block.
Compare it with loading a class or a method.

[EDIT] Moreover, you run the risk that the same code will be used in different places.

HLeithner · 2020-11-29T10:01:52Z

Hardcoding an image into an unoverrideable php file is not a good solution.

sandewt · 2020-11-30T14:19:17Z

@sandewt Same for your vote plugin, You will have to modify it to use what you had already done before this merge.

Done. See [4.0] Improvement Vote Plugin #31098

[EDIT]

infograf768 · 2020-12-01T09:01:52Z

Hardcoding an image into an unoverrideable php file is not a good solution.

Let's say we use file_get_content in AdditionalLoginButtons.php to make it simpler for svg =>

Would'nt the override be possible by replacing $button['image'] when the plugin is enabled by a custom image in the template? We have:

<?php elseif (!empty($button['image'])) : ?>
					<?php echo HTMLHelper::_('image', $button['image'], Text::_($button['tooltip'] ?? ''), [
						'class' => 'icon',
					], true) ?>

by adding a specific conditional
$plugin = PluginHelper::getPlugin('system', 'webauthn');
if ($plugin etc.

N6REJ · 2020-12-01T09:38:56Z

@infograf768 @C-Lodder @HLeithner does this solve everyone's concerns? #31545

Revert "[4.0] Webauth inline SVG button with fill (#31190)"

f277d7e

This reverts commit 31d2bdf.

joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Nov 28, 2020

infograf768 mentioned this pull request Nov 28, 2020

[4.0] htmlhelper svg #31410

Closed

sandewt mentioned this pull request Nov 30, 2020

[4.0] Improvement Vote Plugin #31098

Merged

N6REJ mentioned this pull request Dec 1, 2020

[4.0] modify #31190 #31545

Merged

N6REJ closed this Dec 9, 2020

HLeithner deleted the revert-31190-webauth-frontend branch January 5, 2021 22:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "[4.0] Webauth inline SVG button with fill"#31516

Revert "[4.0] Webauth inline SVG button with fill"#31516
infograf768 wants to merge 1 commit into4.0-devfrom
revert-31190-webauth-frontend

infograf768 commented Nov 28, 2020

Uh oh!

infograf768 commented Nov 28, 2020

Uh oh!

N6REJ commented Nov 28, 2020

Uh oh!

C-Lodder commented Nov 29, 2020

Uh oh!

infograf768 commented Nov 29, 2020

Uh oh!

infograf768 commented Nov 29, 2020 •

edited

Loading

Uh oh!

sandewt commented Nov 29, 2020 •

edited

Loading

Uh oh!

HLeithner commented Nov 29, 2020

Uh oh!

sandewt commented Nov 30, 2020 •

edited

Loading

Uh oh!

infograf768 commented Dec 1, 2020

Uh oh!

N6REJ commented Dec 1, 2020 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

infograf768 commented Nov 28, 2020

Uh oh!

infograf768 commented Nov 28, 2020

Uh oh!

N6REJ commented Nov 28, 2020

Uh oh!

C-Lodder commented Nov 29, 2020

Uh oh!

infograf768 commented Nov 29, 2020

Uh oh!

infograf768 commented Nov 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sandewt commented Nov 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HLeithner commented Nov 29, 2020

Uh oh!

sandewt commented Nov 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infograf768 commented Dec 1, 2020

Uh oh!

N6REJ commented Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

infograf768 commented Nov 29, 2020 •

edited

Loading

sandewt commented Nov 29, 2020 •

edited

Loading

sandewt commented Nov 30, 2020 •

edited

Loading

N6REJ commented Dec 1, 2020 •

edited

Loading