Adds a dismiss button to remove Recent tabs (#177) #248
Conversation
addon/popup.js
Outdated
| @@ -73,8 +73,10 @@ function renderTabList(tabs, containerSelector, eventLabel) { | |||
| let li = document.createElement("li"); | |||
| let image = document.createElement("span"); | |||
| let text = document.createElement("span"); | |||
| let dismiss = document.createElement("span"); | |||
There was a problem hiding this comment.
I think you should use a button here, for accessibility, so it is possible to use the tab key to select this element and the space bar to press it.
addon/popup.js
Outdated
| @@ -83,7 +85,7 @@ function renderTabList(tabs, containerSelector, eventLabel) { | |||
| image.style.backgroundImage = `url(${favIconUrl})`; | |||
| } | |||
| renderedInfo += favIconUrl + " "; | |||
| let anchor = document.createElement("button"); | |||
There was a problem hiding this comment.
Same here, if this is a button then it can be navigated to using the keyboard. Switching it to anything else will regress the bug I fixed by switching this to a button.
addon/popup.css
Outdated
| text-align: center; | ||
| } | ||
|
|
||
| #panel.dark-theme .tab__dismiss::after { |
There was a problem hiding this comment.
I know there is a bit of dark theme stuff here, but that is fine. Make sure it all integrates smoothly when fixing the conflicts in the dark theme pr.
addon/popup.css
Outdated
| position: absolute; | ||
| font-size: 18px; | ||
| color: rgb(83, 85, 85); | ||
| content: '\d7'; /* cross symbol */ |
There was a problem hiding this comment.
https://design.firefox.com/icons/viewer/#close
our "x" is slightly different
htwyford
force-pushed
the
177-dismissRecents
branch
from
49b63ab to
88ad108
Compare
|
Restored both buttons to being One caveat: the 10px to the right of the dismiss symbol and the 3px above and below are not clickable. This is owing to the fact that the main "tab" Thoughts on this? I could make it so that the clickable area of the dismiss button extends above, below and to the right of its visible area, but that might be more confusing. I'm not sure of a way to make the tab button clickable behind the dismiss button while keeping them both |
|
could the |
htwyford
force-pushed
the
177-dismissRecents
branch
from
88ad108 to
220d797
Compare
htwyford
force-pushed
the
177-dismissRecents
branch
from
220d797 to
b1e20a3
Compare
|
Thanks! I did it that way. I spun the event listener out into its own private method to event too much needless duplication. |
|
I need to take another look at this. |
|
💯 |
Addresses #177. Adds a dismiss button to recent tabs that appears on hover.
Adding a button to the tabs required the
anchorelement to be changed from abuttonto a clickablediv.Dark theming in
popup.cssdepends on dark theme #239 being implemented.