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

Allow the Emulator user to specify a User ID #1456

Merged
merged 12 commits into from
Apr 26, 2019

Conversation

denscollo
Copy link
Contributor

@denscollo denscollo commented Apr 24, 2019

Fixes #1344

Description

This PR allows the Emulator users to specify a User ID which will be used within conversations.

Changes made

Functionality changes

To implement this feature we made the following changes:

  • Add an input to the Emulator Settings so that the user can configure an ID
  • Change the startNewConversation method to choose between a randomly generated UserId, the UserId set in the props or the UserId set by the user.
  • When the Use custom id checkbox is set to false, the input is disabled and cleaned. When true, it is a required field and it gets enabled.
  • The User ID field has a validation so that the user is not able to enter anything other than a GUID.
  • Restarting a conversation with the same userId will choose between the one set by the user or the one set by default (if the first one is empty).
  • Restarting a conversation with a new userId will generate a random ID the same way it was doing it until now.
  • Remove Restart Conversation Split Button
  • Add two new buttons Restart with Same User Id and Restart with New User Id to replace Restart Conversation Split Button.
  • Add validation to the Save button to avoid setting the require user ID flag as true but not setting an ID

UI Changes

Before
imagen
imagen

After
imagen
imagen

Testing

Manual testing
BETranscript-Test

Automated testing
Added automated tests for the new settings options.

imagen

@coveralls
Copy link

coveralls commented Apr 24, 2019

Coverage Status

Coverage increased (+0.02%) to 57.053% when pulling ece6fb4 on feature/southworks/userid into 1185b06 on master.

Copy link
Contributor

@justinwilaby justinwilaby left a comment

Choose a reason for hiding this comment

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

Some small changes to consider. Overall it looks good

Copy link
Contributor

@tonyanziano tonyanziano left a comment

Choose a reason for hiding this comment

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

Some styling issues:

The "User ID" input field label needs to have disabled styling (lighter color) when the input field is disabled.

image

Right now, the color makes the input appear to be active when it is actually disabled. The label, "User ID," should use the same color treatment as the checkbox label under "Data Collection" when it is disabled:

image

This feedback will make it easier for users to understand that the checkbox is actually enabling / disabling the text field.


Also, I've spoken to @mewa1024 about the "Restart Conversation" button in the livechat toolbar, and we have decided to keep it as one button with an alternative option via dropdown, the way it was previously. So it should be changed back to a single <SplitButton> instead of two separate buttons.

image

@Aliandi
Copy link
Contributor

Aliandi commented Apr 25, 2019

Ok @justinwilaby and @tonyanziano ! We are working on the feedback!

@tonyanziano
Copy link
Contributor

tonyanziano commented Apr 25, 2019

The text field label looks good when disabled, but the disabled state persists when the text field is enabled. The color of the label should change depending on the state of the text field.

image

Right now:

disabled -> light gray
enabled -> same light gray

Desired behavior:

disabled -> light gray
enabled -> darker gray

@denscollo
Copy link
Contributor Author

Hi @tonyanziano We are still working on the Label Color feedback that you gave us and the one about the getDerivedStateFromProps function. As for the User Id Label, we are having some issues with the disabled property of the label but we are working on it.

@tonyanziano
Copy link
Contributor

Sounds good! @denscollo 👍

Copy link
Contributor

@justinwilaby justinwilaby left a comment

Choose a reason for hiding this comment

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

Some very small housekeeping issues.

justinwilaby
justinwilaby previously approved these changes Apr 26, 2019
@tonyanziano
Copy link
Contributor

Nice job, thanks!

@justinwilaby justinwilaby merged commit 4422c69 into master Apr 26, 2019
@justinwilaby justinwilaby deleted the feature/southworks/userid branch April 26, 2019 21:24
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.

Allow the Emulator user to specify a User ID to be used for conversations
5 participants