? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
24 May 2018

Pull Request for Issue # .

Summary of Changes

  • Introduce a polyfill for the next generation css feature ::focus-visible. That makes flying focus redundant. Read all about it here: https://wicg.github.io/focus-visible/demo/

  • Also removes jQuery migration script

  • and cleanup the dependencies in package.json

Testing Instructions

There is nothing to test here as this is just the polyfill (apart the fact that is loaded correctly in the backend template).
screen shot 2018-05-24 at 16 21 40

There is some css classes/sudo elements that needs to be implemented, eg:

/*
  This will hide the focus indicator if the element receives focus via the mouse,
  but it will still show up on keyboard focus.
*/
.js-focus-visible :focus:not(.focus-visible) {
  outline: none;
}

@ciar4n and me will try to sort this out on another PR

@wilsonge @laoneo @mbabker can we merge this one review?

avatar dgrammatiko dgrammatiko - open - 24 May 2018
avatar dgrammatiko dgrammatiko - change - 24 May 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 May 2018
Category Administration Templates (admin) JavaScript
avatar dgrammatiko dgrammatiko - change - 24 May 2018
The description was changed
avatar dgrammatiko dgrammatiko - edited - 24 May 2018
avatar dgrammatiko dgrammatiko - change - 24 May 2018
The description was changed
avatar dgrammatiko dgrammatiko - edited - 24 May 2018
avatar brianteeman
brianteeman - comment - 24 May 2018

I see that this pr removes jQuery.migrate.
Isn't that unrelated to this pr and should be units own pr?

avatar dgrammatiko
dgrammatiko - comment - 24 May 2018

In theory yes, in practice no as it will take twice the time to do 2 PRs. Also this is extensively discussed and I think we all agree that jquery migrate is mistakenly included in J4 (usage only for devs, so shouldn't be in the distributed code).

avatar dgrammatiko dgrammatiko - change - 24 May 2018
Labels Added: ?
avatar dgrammatiko
dgrammatiko - comment - 24 May 2018

Anyhow, I've reverted that part, someone with ample of free time can do it...

avatar dgrammatiko dgrammatiko - change - 24 May 2018
The description was changed
avatar dgrammatiko dgrammatiko - edited - 24 May 2018
avatar brianteeman
brianteeman - comment - 24 May 2018

I agree 100% that migrate should go but a pr should only have a single purpose.

This polyfill is not a direct replacement for flying focus as it doesn't visually indicate the movement from one element to another. That's a shame but this is probably the best way to go moving forward

avatar dgrammatiko
dgrammatiko - comment - 24 May 2018

@brianteeman I agree but not for the alpha version. we are making our life hard for no good reason. I mean I don't expect someone to test something here. Neither I would expect someone to test a PR for removal of jQuery-migrate, because quite frankly there's nothing to test...

avatar dgrammatiko
dgrammatiko - comment - 24 May 2018

it doesn't visually indicate the movement

Actually the removal of that particular feature is welcome, as it breaks A11Y rules for animations

avatar brianteeman
brianteeman - comment - 24 May 2018

It doesn't or webaim wouldn't use it :)

On Thu, 24 May 2018, 15:54 dGrammatiko, notifications@github.com wrote:

it doesn't visually indicate the movement

Actually the removal of that particular feature is welcome, as it breaks
A11Y rules for animations


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

avatar dgrammatiko
dgrammatiko - comment - 24 May 2018

You do realise that it's a quite old concept: https://web.archive.org/web/20140103103024/http://webaim.org/
That's from way back in 2014

avatar C-Lodder
C-Lodder - comment - 25 May 2018

Good luck. Tried and failed already: #15136

avatar brianteeman
brianteeman - comment - 25 May 2018

I have tested this item successfully on 31fd476


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

avatar brianteeman brianteeman - test_item - 25 May 2018 - Tested successfully
avatar zwiastunsw
zwiastunsw - comment - 29 May 2018

I have tested this item successfully on 31fd476


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

avatar zwiastunsw zwiastunsw - test_item - 29 May 2018 - Tested successfully
avatar Quy Quy - change - 29 May 2018
Status Pending Ready to Commit
avatar Quy
Quy - comment - 29 May 2018

RTC


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

avatar dgrammatiko
dgrammatiko - comment - 30 May 2018

What are the consequences when we remove them here?

@laoneo the command node build.js --update will have the same result as before, so that is the proof of
A. nothing breaks here
B. these are wrongly mentioned as dependencies since we're doing our own versioning here instead of copying from the source (read my comment above)

avatar laoneo laoneo - change - 30 May 2018
Labels Added: ?
avatar laoneo laoneo - change - 30 May 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-05-30 13:15:34
Closed_By laoneo
avatar laoneo laoneo - close - 30 May 2018
avatar laoneo laoneo - merge - 30 May 2018

Add a Comment

Login with GitHub to post a comment