xmonad / xmonad-contrib

Contributed modules for xmonad
https://xmonad.org
BSD 3-Clause "New" or "Revised" License
589 stars 274 forks source link

XMonad.Actions.GridSelect: added gs_cancelOnEmptyClick field #894

Closed sylecn closed 5 months ago

sylecn commented 5 months ago

Description

In the original code, when a GridSelect is shown, user has to use a keyboard to cancel it (ESC key by default). With this field added, when it is set to True, mouse click on empty space can cancel the GridSelect.

When user trigger a GridSelect action using mouse, this change makes it easier to cancel the Grid, without reaching to the keyboard.

Checklist

myGoToSelected = goToSelected def { gs_cancelOnEmptyClick = True }

-- try it with a key binding , ("M-", myGoToSelected)



  - [x] I updated the `CHANGES.md` file
sylecn commented 5 months ago

Hi slotThe, thanks for the review.

I fixed some issues you mentioned and force pushed. Remaining two issues:

  1. about whenMaybe. whenMaybe is in extra package, which is not currently in dependency list. Do you want to add it as a dependency, or just add a local definition?
  2. how to use whenMaybe here? A quick look on the doc gives the following code which doesn't type check.
    Nothing -> whenMaybe (not $ gs_cancelOnEmptyClick gsconfig) contEventloop
    xmonad-contrib>     • Couldn't match type ‘a’ with ‘Maybe a’
    xmonad-contrib>       Expected: TwoD a a
    xmonad-contrib>         Actual: TwoD a (Maybe a)
    xmonad-contrib>       ‘a’ is a rigid type variable bound by
    xmonad-contrib>         the type signature for:
    xmonad-contrib>           stdHandle :: forall a.
    xmonad-contrib>                        Event -> TwoD a (Maybe a) -> TwoD a (Maybe a)
  3. since the change is moved to "Breaking Changes" section, would you say it's okay to set gs_cancelOnEmptyClick to True by default? Which value is a better default?
slotThe commented 5 months ago
1. about whenMaybe. whenMaybe is in extra package, which is not currently in dependency list. Do you want to add it as a dependency, or just add a local definition?

Oops, I thought we have this in ManageHook. Nevermind then, you don't have to create an extra definition for this.

3. since the change is moved to "Breaking Changes" section, would you say it's okay to set gs_cancelOnEmptyClick to True by default? Which value is a better default?

You don't have to necessarily move the whole thing to breaking changes, just the part that's actually a breaking change. I'd say this is can be a reasonable default

sylecn commented 5 months ago

I set the default value to True and updated the doc accordingly.

slotThe commented 5 months ago

Thanks!