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

feat(button): Add pressed prop to place button in active state #715

Merged
merged 17 commits into from Jul 21, 2017

Conversation

Projects
None yet
3 participants
@tmorehouse
Member

tmorehouse commented Jul 20, 2017

Adds a new prop pressed, which defaults to null.

  • If pressed === true then the .active class is added to the button and sets aria-pressed="true" attribute.
  • If pressed === false then the attribute aria-pressed="false" is set.
  • If pressed === null (the default, or any other value) then neither the class .active nor the attribute aria-pressed are set.

Useful when creating toggleable buttons.

The pressed state can be synced by setting the .sync modifier on the prop:

<template>
  <b-btn :pressed.sync="myPressedState">Toggle Me</b-btn>
</template>
<script>
export default {
  data: {
    myPressedState: false
  }
}
</script>

@tmorehouse tmorehouse added this to the v0.19.0 milestone Jul 20, 2017

@tmorehouse tmorehouse requested review from alexsasharegan and mosinve Jul 20, 2017

tmorehouse and others added some commits Jul 20, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

One feature I need to add is set the focus class when the button is clicked (focused) and remove the class on blur. This focus class would only occur if pressed is either true or false, but not null.

This would be done by adding a conditional focus and blur listeners on the button element

@tmorehouse tmorehouse changed the title from feat(buton): Add pressed prop to place button in active state to [WIP] feat(buton): Add pressed prop to place button in active state Jul 20, 2017

@tmorehouse tmorehouse changed the title from [WIP] feat(buton): Add pressed prop to place button in active state to [WIP] feat(button): Add pressed prop to place button in active state Jul 20, 2017

@mosinve

This comment has been minimized.

Member

mosinve commented Jul 20, 2017

About focus class, theres already applies pseudo-class :focus on focus... and it goes away on click (blur) at another clickable element. it done by browser.
As for me there is no need to to browser's work.

mosinve added some commits Jul 20, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

I'm not sure why BS V4 adds the class. They don't document it, but if you inspect the button in pressed state they add 'focus' class when it has focus. maybe it is a just-in-case if the button is rendered as a link or label?

https://github.com/twbs/bootstrap/blob/33715a73d2eae3865cb4c1e0a13d1da4b6aeb278/js/src/button.js#L163-L166

[button] Add focus class for toggle buttons
When pressed is true or false, add the focus class when focused.

Also sets tab-index to -1 when link is disabled to prevent tab from focusing this element, as links do not respect disabled attribute.
@alexsasharegan

This comment has been minimized.

Contributor

alexsasharegan commented Jul 20, 2017

This is looking good so far. I just pulled and ran the docs site to check it out. Maybe helpful to see two btn variants side-by-side to see the pressed stuff in action. Also, maybe a btn-group example?

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

Yeah, I have a toggle style, and a pressed style in the docs. but could easily add a non-pressed one for comparison.

tmorehouse added some commits Jul 20, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

Added in example of non-pressed button beside a pressed button.

Also added in short example for disabled state.

I suppose we could dd in an example of button rendered as a link (with prop href set)

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

Added a couple more examples (regular button and button rendered as a link)

@tmorehouse tmorehouse changed the title from [WIP] feat(button): Add pressed prop to place button in active state to feat(button): Add pressed prop to place button in active state Jul 20, 2017

@tmorehouse tmorehouse added Status: WIP and removed Status: WIP labels Jul 20, 2017

[button] button-pressed focus handler
Creates a single handler used by all instance of button that have the pressed attribute

Removing the need for a focus state data && handler on component.

Global handlers will only be instantiated once, regardless of hoe many times button.vue is imported.
@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 20, 2017

Removed the focus handler from within the component (negating the need to maintain hasFocus state) by creating a global handler for all instances of button which have the toggle/pressed option.

@tmorehouse tmorehouse removed the Status: WIP label Jul 20, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 21, 2017

@alexsasharegan I have added in a button group example for toggle buttons

@mosinve

This comment has been minimized.

Member

mosinve commented Jul 21, 2017

Maybe to add example with v-for generator of buttons?

HTML

<b-button-group>
    <b-button v-for="(button, index) in buttons" :pressed.sync="button.state" :variant="button.variant">
        {{button.caption}}
    </b-button>
</b-button-group>
{{buttons}}

JS

data: {
    buttons: [
      {
        variant: 'primary',
        caption: 'Toggle 1',
        state: null
      },
	  {
        variant: 'primary',
        caption: 'Toggle 2',
        state: false
      },
	  {
        variant: 'primary',
        caption: 'Toggle 3',
        state: false
      },
	  {
        variant: 'primary',
        caption: 'Toggle 4',
        state: false
      },
	  {
        variant: 'primary',
        caption: 'Toggle 5',
        state: false
      }
    ]
}

https://jsfiddle.net/cihkem/fja8kvdr/
or
https://jsfiddle.net/cihkem/fja8kvdr/1/ (without captions at data object)

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Jul 21, 2017

@mosinve good idea. I've update the button-group example.

{ variant: 'warning', caption: 'Toggle 3', state: true },
{ variant: 'success', caption: 'Toggle 4', state: false },
{ variant: 'outline-success', caption: 'Toggle 5', state: false },
{ variant: 'outline-primary', caption: 'Toggle 6', state: false }

This comment has been minimized.

@mosinve

mosinve Jul 21, 2017

Member

One of them could have state = null to show that in this case state is not changing on click

This comment has been minimized.

@tmorehouse

tmorehouse Jul 21, 2017

Member

True... although the name on the button should be "can't toggle me" instead of "toggle #" so people don't get confused.

This comment has been minimized.

@mosinve

mosinve Jul 21, 2017

Member

Yep 👍

This comment has been minimized.

@tmorehouse

tmorehouse Jul 21, 2017

Member

I changed the caption to "No Toggle" for this button 😃

This comment has been minimized.

@mosinve

mosinve Jul 21, 2017

Member

It's almost like "Can't toggle me", so nevermind 😃

This comment has been minimized.

@tmorehouse

mosinve and others added some commits Jul 21, 2017

Small update
make one of button's state `=null` to show that in this case state is not updating after click

@tmorehouse tmorehouse merged commit 61a104f into master Jul 21, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@tmorehouse tmorehouse deleted the tmorehouse-button branch Jul 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment