span class="input-group-append

This should be a button not a span

In every layout?

Yes. You cannot focus on a span

Can we do that in it's own pr, I don't want to mix problems.

Fyi. When I change it then it looks like

as a span it is wrong and not accessible. You cannot navigate to it using the keyboard and you cannot activate it. Try to tab to the span - you cant.

To be honest the way this JS works appears to be a hard coding to a specific markup. I thought the entire point of layouts etc was to free a designer from specific hard requirements like this. We're just swapping one well supported and cohesive framework for a hotch potch of hacked together scripts that still place restrictions.

In the code here it is hard coded to use specific icons and I cannot change that. If as a designer I decide I want to use a padlock instead of an eye then I need to override the markup (expected) but also to replace the javascript otherwise the javascript will replace my padlock with an eye

To help you understand better (I hope) this is a movie of the signup at gmail with the text displayed from the output of a screenreader as I use my keyboard to navigate the screen

Nobody says, that it doesn't need to be moved to a button. But it needs some styling then where I'm the wrong person for that. So better to do that in it's own pr.
Regarding the hard coded icons. I agree. We can do it the google way, to load two elements and then disable the one which is not needed. Or even load the icons inline as google does, where we have a pr ready joomla/40-backend-template#441.

any markup changes should go here - css changes can be separated if required.

While talking about required we need to remove the "required" attribute from both the username and password in the admin login. Otherwise you expose a really annoying html5/screen reader bug see the first video here http://usability.com.au/2013/05/accessible-forms-2-required-fields-and-extra-information/

To be clear. The markup needs to be in this PR otherwise we run the risk of changing markup later and breaking the javascript again.

Better to get the button and the hard coded icons resolved here and if required the css can be done later if you insist - at least that shouldnt break the js

@laoneo you should really revive #19231 for this. All fields needs to be custom elements or you have to make them compatible with repeatable (add custom events etc)

@dgrammatiko you closed it in a broken state :(
#19231 (comment)

Because I started that thingy on December last year posted tasks that other people could help and then after 8 months that nobody cared to decouple the stupid bootstrap and font awesome and nobody did an a11y audit I closed it. I guess you should blame me for that as well, I mean I should have patched all the things myself...

Read your last comment on that issue.

Even if that is true, probably was if I wrote it (I mean how many times someone can rebase a PR without actually breaking it?), will not change the status of that PR. Still needed decoupling from BS and FA and an a11y audit.

@dgrammatiko atm I have no time to also take care of CE. I try to bring J4 more or less into a stable state. The global service provider task is half baked, when this is done then I can shift my focus. But the PHP side needs some more love.

@brianteeman the code is not bound to the types of elements. How it is now, it works only with classes and the structure. It has also no dependency to font awesome classes anymore. Really, the button change I would suggest to do it in it's own pr. I don't want to create a pr which in itself is not finished and then we will have the risk to forget the styling pr.

It is hard coded to use specific fonts as I showed. Its not hard to resolve that with generic class names and CSS. Guess I will have to do that in a separate pr when I make it accessible.

@brianteeman NO you are wrong! Read the closing comment on
joomla/40-backend-template#441 Everything that renders (also) on the front end NEEDS to have svg icons and not be coupled to Font Awesome. Not my decision, @wilsonge' and the rest of the front end team. So you shouldn't use fa fa-something on all the form fields that could also be used in the front end...

We lost focus in his pr as it is a bug fix for a current javascript error. I'v opened the issue #22680 which addresses the concerns of @dgrammatiko. The markup changes requested from @brianteeman are already covered in issue #22543.

No idea what you are talking about Dimitris. This pr as it is right is is
hard coded to use specific font icons. It is that hard requirement to
specific font icons I am saying should be removed. You should be saying
thank you Brian not attacking me for doing the very thing you have been
shouting for in the last two years.

On Wed, 17 Oct 2018, 10:08 Allon Moritz, notifications@github.com wrote:

We lost focus in his pr as it is a bug fix for a current javascript error.
I'v opened the issue #22680
#22680 which addresses the
concerns of @dgrammatiko https://github.com/dgrammatiko. The markup
changes requested from @brianteeman https://github.com/brianteeman are
already covered in issue #22543
#22543.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#22666 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8SHw6cpbq4dZht26OlEdKvkrIGzOks5ulvN2gaJpZM4XePcq
.

You shouldn't couple the form icons with Font Awesome eg don't use fa fa-whatever...

Well in this PR we have removed the fa class as I can see. There's open issues to track the comments raised here (clearly more to be done) but we've done the es6 migration here in it's own right so I'm going to merge this so we can continue making other improvements

it is hard coupled to specific icons - thats my point. I wish people would read instead of assuming.

Well in this PR we have removed the fa class as I can see

Not really because the classes are just extending the fa fa-whatever:

@brianteeman the solution for the js was already pointed out from @laoneo

Regarding the hard coded icons. I agree. We can do it the google way, to load two elements and then disable the one which is not needed.

My comment was about removing the font awesome coupling so I'm not getting what you think I'm assuming here

lol