Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Changes the close button to an actual button#5991

Merged
darkwing merged 8 commits into
firefox-devtools:masterfrom
BadInternetGuy:close-button
Apr 17, 2018
Merged

Changes the close button to an actual button#5991
darkwing merged 8 commits into
firefox-devtools:masterfrom
BadInternetGuy:close-button

Conversation

@BadInternetGuy
Copy link
Copy Markdown
Contributor

Fixes Issue: #4070

This changes to div to a button, so that it can work with a screen reader. The padding around the button is still slightly to large, but the button works as needed.

Ctrl+Shift+F to open a search bar.
Press the close button.
Padding makes the button slightly to large.

clickedbutton

@darkwing
Copy link
Copy Markdown
Contributor

I added two commits:

  1. Update to jest tests (yarn jest -u)
  2. Lint fixes (yarn lint)

@darkwing
Copy link
Copy Markdown
Contributor

This is looking really good! A few notes:

  • The button looks wider than it was before, which is easy to see when you hover over it; <button> elements are given padding by default in every browser so I think the fix is:
.close-btn {
  padding: 0;
}

I looked at the update in Chrome (we don't officially support using the debugger in Chrome but we do our best) and the "x" somehow doesn't display...until my fix above. Padding saves the day!

Once you update padding I think this is good to go! :) . Make sure to pull down my changes git pull origin close-button before making updates! Cheers!

Copy link
Copy Markdown
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

I decided to do the honors of removing padding; once tests pass I'll merge. Thank you so much!

@darkwing darkwing merged commit 9daaf6e into firefox-devtools:master Apr 17, 2018
jasonLaster pushed a commit that referenced this pull request Apr 18, 2018
* made a test change

* Fixed README

* fixed the close button

* fixed the close button

* changed the div to button in the test cases

* Update jest snapshots

* Linting fixes

* Fix padding
jasonLaster pushed a commit that referenced this pull request Apr 18, 2018
* made a test change

* Fixed README

* fixed the close button

* fixed the close button

* changed the div to button in the test cases

* Update jest snapshots

* Linting fixes

* Fix padding
@Malachi-Holden Malachi-Holden deleted the close-button branch April 18, 2018 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants