DOMUI Prefs: Change subpage close button to an actual button element

DOMUI Prefs: Change subpage close button to an actual button element


BUG=None
TEST=Close button on DOMUI prefs subpages should still work.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68391

[arv (Not doing code reviews), 2010-12-06 22:06:02]

LGTM

Do you want tabIndex="-1" to prevent the user from tabbing to the button?

erik



On Mon, Dec 6, 2010 at 14:02,  <stuartmorgan@chromium.org> wrote:
> Reviewers: arv,
>
> Description:
> DOMUI Prefs: Change subpage close button to an actual button element
>
>
> BUG=None
> TEST=Close button on DOMUI prefs subpages should still work.
>
> Please review this at http://codereview.chromium.org/5568007/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/resources/options.html
>  M chrome/browser/resources/options/options_page.css
>
>
> Index: chrome/browser/resources/options.html
> diff --git a/chrome/browser/resources/options.html
> b/chrome/browser/resources/options.html
> index
>
eb6448ccd95f33e0bed7b3de91e0cde7284e9e17..95eb58f26d45b4316f740448eb3fd608dc248e60
> 100644
> --- a/chrome/browser/resources/options.html
> +++ b/chrome/browser/resources/options.html
> @@ -337,8 +337,8 @@ window.onpopstate = function(e) {
>        <!-- End main pages -->
>        <div id="subpage-sheet-container" class="hidden">
>          <div id="subpage-sheet">
> -           <div class="close-subpage"
> -               onclick="OptionsPage.closeSubPage();"></div>
> +           <button class="close-subpage"
> +               onclick="OptionsPage.closeSubPage();"></button>
>            <if expr="pp_ifdef('chromeos')">
>              <include src="options/about_page.html">
>              <include src="options/chromeos_language_options.html">
> Index: chrome/browser/resources/options/options_page.css
> diff --git a/chrome/browser/resources/options/options_page.css
> b/chrome/browser/resources/options/options_page.css
> index
>
f009d53fcdb6a64e302454676a5ba89122b96dee..4ab0d56d3ec6f923e4047b86dec249231e43b65d
> 100644
> --- a/chrome/browser/resources/options/options_page.css
> +++ b/chrome/browser/resources/options/options_page.css
> @@ -39,8 +39,10 @@ body {
>  }
>
>  .close-subpage {
> +  background-color: transparent;
>   /* TODO(stuartmorgan): Replace with real images once they are available.
> */
>   background-image: url("../../../app/theme/close_bar.png");
> +  border: none;
>   position: relative;
>   top: 12px;
>   height: 16px;
>
>
>

[stuartmorgan, 2010-12-06 22:11:00]

If someone is tabbing through the controls, it seems like having a way to close
the subpage from the tab rotation is probably a win.

[arv (Not doing code reviews), 2010-12-06 22:16:32]

On Mon, Dec 6, 2010 at 14:11,  <stuartmorgan@chromium.org> wrote:
> If someone is tabbing through the controls, it seems like having a way to
> close
> the subpage from the tab rotation is probably a win.

Good point.

I also hope that escape works?

-- 
erik

[stuartmorgan, 2010-12-06 22:25:32]

On Mon, Dec 6, 2010 at 2:16 PM, Erik Arvidsson <arv@chromium.org> wrote:
> I also hope that escape works?

Not yet, but I'll double-check that we have a bug about wiring it up
so it's not forgotten.

-Stuart