clients
clients copied to clipboard
[PS-1222] Remove `appBlurClick` throughout the popup and web code
Type of change
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
appBlurClick
leads to focus being lost/reset for assistive technology users. It should not be necessary in any case - if focus does need to move after an action, explicitly set it somewhere programmatically using focus()
rather than relying on browser heuristics. in cases where the view completely changes as a result of an action, focus is reset automatically anyway.
this is a follow-up to similar previous PRs (both in the browser extension and desktop app) https://github.com/bitwarden/clients/pull/2662 / https://github.com/bitwarden/clients/pull/2654
Code changes
- removed
appBlurClick
throughout the entirety of the codebase (too many files to list explicitly here)
Screenshots
No discernible change in UI, nor - from my testing - in behaviour.
Before you submit
- [x] I have checked for **linting** errors (`npm run lint`) (required)
- [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required)
- [ ] This change requires a **documentation update** (notify the documentation team)
- [ ] This change has particular **deployment requirements** (notify the DevOps team)
Thank you for your contribution! We've added this to our internal Community PR board for review. ID: PS-1222
Recommend thoroughly testing all functionality of the popup and web after this change. From my own testing, I have not noticed any negative behaviours.
After merging master branch back into this, it now seems to only be touching on two straggler appBlurClick
s ... seems the other files have already been fixed/have removed it, which is nice to see
As we are no longer using it, we should also remove the directive
Is it enough to remove that file, or are there deeper ties anywhere else for it? (I'll have a little search, as sadly I'm still not really an angular guy, I just poke it from the side and see what happens)
As we are no longer using it, we should also remove the directive
Is it enough to remove that file, or are there deeper ties anywhere else for it? (I'll have a little search, as sadly I'm still not really an angular guy, I just poke it from the side and see what happens)
After removing the file with the directive, the registration of that directive needs to be removed from jslib.module.ts
@djsmith85 okidoki, i'll give it a go and add it to this PR
@djsmith85 done I think
@djsmith85 done I think
Almost..., you've removed the import for the directive but it's still used in jslib.module.ts
(twice)
serves me right for not testing...should be fixed now
wunderbar, thank you @djsmith85 (and sorry for the flood of PRs ... was clearly on a roll/bored recently)
No problem at all and every contribution is appreciated. Keep up the great work, seems you'll be leading community efforts again for 2022.09 😉