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

Font manager - tabIndex attribute enhancement #1279

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

debbieellis7
Copy link
Contributor

@debbieellis7 debbieellis7 commented Apr 28, 2021

Description

Issue: #1206

Testing instructions

Screenshots

Checklist:

  • I've tested the code.
  • My code is easy to read, follow, and understand
  • My code follows the accessibility standards.
  • My code has proper inline documentation / docblocks.

Additional Comments

@debbieellis7 debbieellis7 linked an issue Apr 28, 2021 that may be closed by this pull request
@debbieellis7 debbieellis7 changed the title tabIndex attribute enhancement Font manager - tabIndex attribute enhancement Apr 28, 2021
@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #1279 (627ab13) into development (00d1164) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@                Coverage Diff                @@
##             development    #1279      +/-   ##
=================================================
- Coverage          23.35%   23.34%   -0.01%     
  Complexity          2876     2876              
=================================================
  Files                242      242              
  Lines               9807     9806       -1     
  Branches             364      361       -3     
=================================================
- Hits                2290     2289       -1     
  Misses              7509     7509              
  Partials               8        8              
Impacted Files Coverage Δ
.../assets/js/react/components/FontManager/AddFont.js 100.00% <ø> (ø)
...eact/components/FontManager/AddUpdateFontFooter.js 87.93% <ø> (ø)
...ets/js/react/components/FontManager/FontManager.js 72.22% <ø> (ø)
...ets/js/react/components/FontManager/FontVariant.js 88.46% <ø> (ø)
...ssets/js/react/components/FontManager/SearchBox.js 91.66% <ø> (ø)
...rc/assets/js/react/components/Modal/CloseDialog.js 100.00% <ø> (ø)
...s/js/react/components/FontManager/FontListItems.js 86.86% <100.00%> (+0.13%) ⬆️
...js/react/components/FontManager/FontManagerBody.js 95.36% <100.00%> (-0.10%) ⬇️
...sets/js/react/components/FontManager/UpdateFont.js 90.00% <100.00%> (+1.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00d1164...627ab13. Read the comment docs.

@debbieellis7 debbieellis7 force-pushed the tab-index-enhancement branch 2 times, most recently from 5cdb7c1 to e56fe08 Compare April 28, 2021 23:59
@@ -333,7 +333,7 @@ export class FontListItems extends Component {
const { disableSelectFontName, deleteId } = this.state
const { id, loading, fontList, searchResult, selectedFont } = this.props
const list = !searchResult ? fontList : searchResult
const tabIndex = updateFontVisible ? '-1' : '144'
const tabIndex = updateFontVisible ? '-1' : '0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@debbieellis7 this line could be removed now, right?

@jakejackson1 jakejackson1 added this to the 6.x milestone May 31, 2021
@jakejackson1
Copy link
Member

@jestonihpi this PR needs to be rebased on development, force pushed, and then tested.

@debbieellis7 debbieellis7 force-pushed the tab-index-enhancement branch from e56fe08 to 0021da0 Compare April 21, 2022 11:11
@debbieellis7 debbieellis7 requested a review from kielllll April 21, 2022 11:12
@debbieellis7
Copy link
Contributor Author

I've just rebased this branch @kielllll

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove all tabindex usage > 0
4 participants