Skip to content

Dialpad first version#1870

Merged
carocao-msft merged 17 commits intomainfrom
carocao/dialpad
May 11, 2022
Merged

Dialpad first version#1870
carocao-msft merged 17 commits intomainfrom
carocao/dialpad

Conversation

@carocao-msft
Copy link
Copy Markdown
Contributor

@carocao-msft carocao-msft commented May 9, 2022 •
edited
Loading

What

Created a dialpad component that allows user to click on the dialpad/ use keyboard to enter numbers

Why

This is a required feature for PTSN calling

How Tested

Tested in storybook
Screen Shot 2022-05-09 at 12 34 44 PM
Screen Shot 2022-05-09 at 12 34 39 PM

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@carocao-msft carocao-msft requested a review from alkwa-msft as a code owner May 9, 2022 19:01
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-zilizcpnml.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-jfttxiyyob.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-wfsnoskduw.chromatic.com/

JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.styles.ts Outdated
JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-nwezxwbken.chromatic.com/

JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-efpvgiqqcc.chromatic.com/

JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/stable-review/react-components.api.md Outdated
JamesBurnside
JamesBurnside reviewed May 9, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/storybook/stories/Dialpad/snippets/CustomDialpad.snippet.tsx
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/react-components/src/localization/locales/en-US/strings.json Outdated
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/react-components/src/components/styles/Dialpad.styles.ts Outdated
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/react-components/src/components/Dialpad/Dialpad.tsx Outdated
@JamesBurnside
Copy link
Copy Markdown
Member

JamesBurnside commented May 10, 2022

Error text is a little unreadable in dark mode:
image

Looks to ahve a different value to sendbox's:
image
image

Can you check out @mgamis-msft's PR to keep these two values consistent: #1861

JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/react-components/stable-review/react-components.api.md Outdated
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/communication-react/stable-review/communication-react.api.md Outdated
JamesBurnside
JamesBurnside reviewed May 10, 2022
View reviewed changes
Comment thread packages/storybook/stories/Dialpad/Dialpad.stories.tsx
@carocao-msft
Copy link
Copy Markdown
Contributor Author

carocao-msft commented May 10, 2022

It's looking great!! One bit of design feedback for @pereiralex - can we have a delete button somewhere? e.g. if I type a number by clicking the dialpad buttons, if I make a mistake currently there's no way to delete an incorrect digit:

image

Checked with Alex P and he said since dialpad is keyboard editable, we are not adding in a delete button for now, just to keep things simple

@carocao-msft
Copy link
Copy Markdown
Contributor Author

carocao-msft commented May 11, 2022

Error text is a little unreadable in dark mode: image

Looks to ahve a different value to sendbox's: image image

Can you check out @mgamis-msft's PR to keep these two values consistent: #1861

Updated color for error text

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-czydcfsmhe.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-ujijynhdnf.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-ymuwbdgbfx.chromatic.com/

JamesBurnside
JamesBurnside reviewed May 11, 2022
View reviewed changes
Comment thread packages/react-components/src/components/utils/formatPhoneNumber.test.ts

import { formatPhoneNumber } from './formatPhoneNumber';

describe('Dialpad numbers should be formatted based on NA phone format by formatPhoneNumber function', () => {
Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside May 11, 2022

Choose a reason for hiding this comment

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

Great tests! 🤩

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-rtrehilvof.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-nxnlkqxjse.chromatic.com/

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2022

Storybook URL https://60c7ae6891f0e90039d7cd54-kflmvbehku.chromatic.com/

@carocao-msft carocao-msft merged commit 009be2d into main May 11, 2022
@carocao-msft carocao-msft deleted the carocao/dialpad branch May 11, 2022 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JamesBurnside JamesBurnside JamesBurnside approved these changes

@dmceachernmsft dmceachernmsft dmceachernmsft approved these changes

@alkwa-msft alkwa-msft Awaiting requested review from alkwa-msft

@anjulgarg anjulgarg Awaiting requested review from anjulgarg

@edwardlee-msft edwardlee-msft Awaiting requested review from edwardlee-msft

@mgamis-msft mgamis-msft Awaiting requested review from mgamis-msft

@PorterNan PorterNan Awaiting requested review from PorterNan

@prabhjot-msft prabhjot-msft Awaiting requested review from prabhjot-msft

@prprabhu-ms prprabhu-ms Awaiting requested review from prprabhu-ms

Assignees

@JamesBurnside JamesBurnside

@dmceachernmsft dmceachernmsft

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@carocao-msft @JamesBurnside @dmceachernmsft
⚡