Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Webauth inline SVG button with fill #31190

Merged
merged 57 commits into from
Nov 12, 2020
Merged

[4.0] Webauth inline SVG button with fill #31190

merged 57 commits into from
Nov 12, 2020

Conversation

N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Oct 21, 2020

Pull Request for Issue #31166

Summary of Changes

changed webauthn button colors to match admin colors

Testing Instructions

go to login in frontend & backend using https://
note webauthn icon is missing/hidden.
apply pr
run npm ci
now both logo and text are visible

Actual result BEFORE applying this Pull Request

image

Expected result AFTER applying this Pull Request

image
image

html will look like this.
image

Documentation Changes Required

show as demo how to use svg inline via $Button using this as example.
creates HTMLHelper::_('icons.svg', $file, $relative = true) class. This creates an inline svg class using either relative to /media or given path/image.

i.e. 'svg' => HTMLHelper::_('icons.svg', "plg_system_webauthn/webauthn-black.svg", true),
will get the image "webauthn-black.svg" from /media/plg_system_webauthn/images/webauthn-black.svg

then to output it you simply need.

<?php if (!empty($button['svg'])): ?>
    <?php echo $button['svg']; ?>
<?php endif; ?>

sample output is

<svg xmlns="http://www.w3.org/2000/svg" width="32" height="32" viewBox="0 0 24 24"><path 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>

see /plugins/system/webauthn/scr/PluginTraits/AdditionalLoginButtons.php for full example

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Oct 21, 2020
@N6REJ N6REJ mentioned this pull request Oct 21, 2020
@PhilETaylor

This comment was marked as abuse.

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 21, 2020

Surely the color of the button should reflect the template button color and not be a darker blue ?

thats kinda what I thought but I don't think they've gotten that far. It's (btn-secondary) is gray on white in cassiopeia right now.
I suppose I could change it to btn-primary.. hopefully one of the cassiopeia folks will chime in.

@sandewt
Copy link
Contributor

sandewt commented Oct 21, 2020

Pull Request for Issue # 31166

Please # 31166 => #31166 for linking

@SharkyKZ
Copy link
Contributor

Maybe use a different image?

@brianteeman
Copy link
Contributor

@PhilETaylor is right -

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 21, 2020

Maybe use a different image?

plugin only supports a single image.

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 21, 2020

done

@C-Lodder
Copy link
Member

Just an FYI, you can inherit the button text colour: https://jsfiddle.net/Ln9xrzfo

@SharkyKZ
Copy link
Contributor

Maybe use a different image?

plugin only supports a single image.

Images can be overridden by templates.

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 22, 2020

Just an FYI, you can inherit the button text colour: https://jsfiddle.net/Ln9xrzfo

only works with inline svg 👎

@C-Lodder
Copy link
Member

Why not use inline SVG then?

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 22, 2020

tbh, cause I don't know how I would do so in the plugin

@sandewt
Copy link
Contributor

sandewt commented Oct 23, 2020

tbh, cause I don't know how I would do so in the plugin

Try this:

protected $app;

public function onUserLoginButtons(string $form): array
{
	if ($this->app->isClient('administrator'))
	{
		$image = 'plg_system_webauthn/webauthn-white.svg'; // backend
	}
	else
	{
		$image = 'plg_system_webauthn/webauthn-color.png'; // site
	}		

	return [
		[
			....
			....
			'image' => $image,
			....
		],
	];

[EDIT] Oops, I thought, you would use a white button at the frontend.

@infograf768
Copy link
Member

To use inline svg in that case, see
#31098

targetting the svg with css let's define the color using a variable

for the vote plugin is used for example:

.content_rating .vote-star svg {
	width: 1em;
	height: 1em;
	fill: #fd7e14;
}

For Cassiopea, can be
$white-offset: #fefefe;

i.e.

.whateverclass svg {
...
...
fill: $white-offset;
}

3rd party templates would just have to modify their css to fit their needs.

@N6REJ
Copy link
Contributor Author

N6REJ commented Oct 23, 2020

NOW WORKS AS SVG INLINE! YAY ME!

@N6REJ N6REJ changed the title Webauth frontend button color Webauth SVG button with fill Oct 23, 2020
@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 7, 2020
@infograf768
Copy link
Member

restarted drone

@infograf768
Copy link
Member

@N6REJ
I suggest to not add the class .plg_system_webauthn_login_button svg in template-rtl.scss in both atum and cassiopea but rather use the direction in icon.scss.

I mean doing this in _icon.scss

  // WebAuthn
  .plg_system_webauthn_login_button svg {
    [dir=ltr] & {
      margin-right: 2px;
    }
    
    [dir=rtl] & {
      margin-left: 2px;
    }
  }

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 8ef4846


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31190.

@infograf768 infograf768 merged commit 31d2bdf into joomla:4.0-dev Nov 12, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 12, 2020
@infograf768 infograf768 added this to the Joomla 4.0 milestone Nov 12, 2020
@infograf768
Copy link
Member

Tks.

@N6REJ N6REJ deleted the webauth-frontend branch November 12, 2020 14:29
@dgrammatiko
Copy link
Contributor

@ infograf768 You've made a huge mistake merging this. The API is incomplete and extremely naive (talking about public static function svg(string $file, bool $relative = true): ?string)...

@richard67
Copy link
Member

The API is incomplete and extremely naive (talking about public static function svg(string $file, bool $relative = true): ?string)...

@dgrammatiko Do you think it can be fixed/improved before the next beta, and if so, could you help with that?

@dgrammatiko
Copy link
Contributor

Do you think it can be fixed/improved before the next beta, and if so, could you help with that?

I'll try to make a PR although pretty busy atm and I have already promised to provide some help in another sub-project from @bembelimen

@infograf768
Copy link
Member

This can be easily reverted if necessary.

@N6REJ
Copy link
Contributor Author

N6REJ commented Nov 16, 2020

@dgrammatiko nice... so I don't do it the way that you abandoned therefore my code is crap. Then you further admit "I'm pretty busy". So your just ragging on me without anything useful to contribute! How beneficial. You had 26 days to make it better!
.

@dgrammatiko
Copy link
Contributor

@N6REJ you know what I'll take it back. Your API is perfect, as the rest of the Joomla. Bye.-

@N6REJ
Copy link
Contributor Author

N6REJ commented Nov 16, 2020

@dgrammatiko I seriously doubt it's perfect, It's functional. Your a good coder but for w/e reason you seem to have made it your lifes mission to trash everything I do WITHOUT giving feedback that is constructive and useful.
as @sandewt & @infograf768 can attest to, I've got no problem with folks saying, "Hey bear, can you fix this please it needs to do xyz cause of................" Thats HELPFUL. Saying I'm naive isn't!

@dgrammatiko
Copy link
Contributor

@N6REJ Check your Glip. You've asked me for help and I provided it but you've completely ignored it. I'm not trashing you without reason: you've started this icons improvements without a plan and you just keep doing small things ignoring B/C or sustainability. If you had a concrete plan that could move the project forward I will back it but that's not what's happening.(how many PR's already for this one icon?)

@N6REJ
Copy link
Contributor Author

N6REJ commented Nov 16, 2020

@dgrammatiko I did as you asked and checked your work that you said wasn't the right way, and didn't understand the discussion. I don't TRY to ignore b/c or anything else. I may not think of it, or realize it & in the case of fa I was under the impression that with a major version change we were allowed to break b/c. I was told no, and why and that started a process to make things HOPEFULLY even better. @hans2103 & myself formed a team specifically for icons and together came up with the recent changes to iconclass that now ensure no 3rd party icons are broken. Was a huge task and ALOT of credit goes to hans.
If you'd like to join the "Bug Squad Icon Team" your perfectly welcome to.
As to doing small things, your 100% correct. Thats what I know how to do and often struggle with that. What takes you minutes takes me days. I won't go into details why but it does.

@sandewt
Copy link
Contributor

sandewt commented Nov 16, 2020

In this way, everyone contributes their own. Only together we are strong.

infograf768 added a commit that referenced this pull request Nov 28, 2020
HLeithner pushed a commit that referenced this pull request Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Required NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet