Skip to content
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: Disabling buttons components once clicked #1245

Merged
merged 4 commits into from
Jan 29, 2021

Conversation

vanbasten17
Copy link
Contributor

@vanbasten17 vanbasten17 commented Jan 12, 2021

Description

  • Disabling buttons once clicked.
  • As discussed, the properties are autodisable and disabledstyle (they are in lowercase to avoid annoying warnings, as these props will be rendered in the server side also).
  • Created a class ButtonsDisabler with static methods inside /util/webchat.js to handle all the related logic of this new feature
  • Added unit tests

Context

We need a way to disable buttons in webchat once they are clicked for some customers

Approach taken / Explain the design

In this case, each button receives a parentId which is used to know what is the parent needed to have their buttons disabled. Once clicked, the message is updated with its buttons set with the property disabled: {true|false}. On next renders, the button will be rendered depending on this property.

To document / Usage example

export const webchat = {
  theme: {
    style: {
      width: 500,
    },
    button: {
      disabledstyle: {
        opacity: 0.5,
        cursor: 'auto',
        pointerEvents: 'none',
        backgroundColor: 'green',
      },
      autodisable: true,
    },
  },
}

Buttons

<Text>
    Here I display two types of buttons, the first one is a URL button and the second is a payload button:
     <Button payload='https://botonic.io' autodisable={false}>Visit botonic.io</Button>
     <Button payload='carousel'>Show me a carousel</Button>
</Text>

Screenshot 2021-01-28 at 12 30 29

Screenshot 2021-01-28 at 12 30 34

Carousel

<Carousel>
          {movies.map((e, i) => (
            <Element key={e.name}>
              <Pic src={e.pic} />
              <Title>{e.name}</Title>
              <Subtitle>{e.desc}</Subtitle>
              {i === 0 ? (
                <Button payload={e.url}>Visit website</Button>
              ) : (
                <Button
                  payload={e.url}
                  autodisable={true}
                  disabledstyle={{ backgroundColor: 'red' }}
                >
                  KAKA
                </Button>
              )}
            </Element>
          ))}
        </Carousel>

Screenshot 2021-01-28 at 12 30 44

Screenshot 2021-01-28 at 12 30 52

As you can observe, properties defined at component level, overwrite the ones defined in the webchat theme. If autodisable is set in webchat.theme, then the components that do not have disabling properties will have the defined ones in the theme (appended in their props).

Testing

  • Has unit tests

@vanbasten17 vanbasten17 marked this pull request as draft January 12, 2021 12:20
@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch 2 times, most recently from d2a0f6a to 562b3e7 Compare January 12, 2021 12:22
Copy link
Contributor

@asastre asastre left a comment

Choose a reason for hiding this comment

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

Looking good! Some things to take into account.

packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/msg-to-botonic.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/components/message.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
@vanbasten17
Copy link
Contributor Author

Nice! I agree with both of you so next points will be:

  • generic property to enable/disable feature
  • specific prop: disabledStyle to enable/disable button specifically (it should take preference over the generic property in case it is defined)
  • Not using DOM operations but directly props that updates the state component with useEffect.
  • Using providers to expose host in case that shadowDOM is enabled.
  • Implementation working with Custom Messages.

@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch 3 times, most recently from 2a8dc09 to d97aeb1 Compare January 18, 2021 14:21
@codecov
Copy link

codecov bot commented Jan 18, 2021

Codecov Report

Merging #1245 (63ec07d) into master (eccc26a) will increase coverage by 0.77%.
The diff coverage is 76.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1245      +/-   ##
==========================================
+ Coverage   64.73%   65.51%   +0.77%     
==========================================
  Files         236      237       +1     
  Lines        6471     6518      +47     
  Branches     1118     1116       -2     
==========================================
+ Hits         4189     4270      +81     
+ Misses       1968     1923      -45     
- Partials      314      325      +11     
Flag Coverage Δ
botonic-react 58.14% <76.56%> (+2.85%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...react/src/components/multichannel/multichannel.jsx 65.38% <ø> (ø)
packages/botonic-react/src/webchat/webchat.jsx 49.56% <ø> (-1.16%) ⬇️
packages/botonic-react/src/components/button.jsx 31.25% <27.27%> (+3.97%) ⬆️
...s/botonic-react/src/components/buttons-disabler.js 81.25% <81.25%> (ø)
packages/botonic-react/src/util/webchat.js 88.88% <88.88%> (ø)
packages/botonic-react/src/components/carousel.jsx 47.16% <100.00%> (+33.70%) ⬆️
packages/botonic-react/src/components/message.jsx 38.18% <100.00%> (+1.14%) ⬆️
packages/botonic-react/src/msg-to-botonic.jsx 47.14% <100.00%> (+6.56%) ⬆️
packages/botonic-react/src/util/objects.js 100.00% <100.00%> (ø)
packages/botonic-react/src/util/react.js 94.73% <100.00%> (+1.87%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eccc26a...63ec07d. Read the comment docs.

@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch 2 times, most recently from 1331499 to 5f69e54 Compare January 22, 2021 14:15
@github-actions
Copy link

Unit Test Results

  1 files    5 suites   15s ⏱️
20 tests 20 ✔️ 0 💤 0 ❌

Results for commit 5f69e54.

@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch 4 times, most recently from ac75133 to cc75b25 Compare January 26, 2021 17:41
@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch from cc75b25 to 83124a1 Compare January 27, 2021 14:35
@vanbasten17 vanbasten17 changed the title wip(react): disabling buttons once clicked Disabling buttons components once clicked Jan 28, 2021
@vanbasten17 vanbasten17 changed the title Disabling buttons components once clicked feat: Disabling buttons components once clicked Jan 28, 2021
@vanbasten17 vanbasten17 marked this pull request as ready for review January 28, 2021 12:13
@vanbasten17 vanbasten17 requested a review from dpinol January 28, 2021 12:22
packages/botonic-react/src/util/webchat.js Outdated Show resolved Hide resolved
packages/botonic-react/src/util/webchat.js Outdated Show resolved Hide resolved
packages/botonic-react/src/util/webchat.js Outdated Show resolved Hide resolved
Copy link
Contributor

@asastre asastre left a comment

Choose a reason for hiding this comment

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

Great work!! Just a few small comments/suggestions

packages/botonic-react/src/components/button.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/util/webchat.js Outdated Show resolved Hide resolved
packages/botonic-react/src/node-app.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/util/webchat.js Outdated Show resolved Hide resolved
@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch from c0174b5 to 6cac414 Compare January 29, 2021 10:14
@vanbasten17 vanbasten17 force-pushed the feat/disable-components branch from 6cac414 to 63ec07d Compare January 29, 2021 10:23
Copy link
Contributor

@asastre asastre left a comment

Choose a reason for hiding this comment

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

🚀

@vanbasten17 vanbasten17 merged commit 8191c40 into master Jan 29, 2021
@vanbasten17 vanbasten17 deleted the feat/disable-components branch January 29, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants