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

[Ext] Create SelectButton Component #20609

Closed
garrettbear opened this issue Aug 25, 2023 · 1 comment · Fixed by #21897
Closed

[Ext] Create SelectButton Component #20609

garrettbear opened this issue Aug 25, 2023 · 1 comment · Fixed by #21897
Assignees
Labels
release-11.8.0 Issue or pull request that will be included in release 11.8.0 release-11.9.0 Issue or pull request that will be included in release 11.9.0 team-design-system All issues relating to design system in Extension

Comments

@garrettbear
Copy link
Contributor

No description provided.

@garrettbear garrettbear added the team-design-system All issues relating to design system in Extension label Aug 25, 2023
@garrettbear garrettbear self-assigned this Aug 25, 2023
garrettbear added a commit that referenced this issue Nov 20, 2023
### Please note that `SelectButton` and `SelectOption` are WIP files but
are needed to make working with SelectWrapper possible.


## Explanation
`SelectWrapper` is the context api/data wrapper for what will be used
with `SelectButton`, `SelectOption`, and other future components. _This
component doesn't have any visual aspects to it, so anything with see
from the UI is temporary_ and will be updated in future PRs (
#20609 and
#20610 )

Fixes #20607 

- There is a lot to digest with the `SelectWrapper` component as it does
have ties to `SelectButton` and `SelectOption`. Something worth pointing
out is there is the option to have **controlled and uncontrolled for
both value and the open state**.
- Controlled open uses props `isOpen` and `onOpenChange`
- Controlled value uses props `defaultValue`, `value`, and
`onValueChange`
- The SelectWrapper is in itself a context api and has a
`useSelectContext` hook that can be utilized when creating custom
features and use controlled props. See the `SelectContextType` to
understand what may be used with this hook.


To Do / WIP Issues: _It's a bit unclear if it really makes sense to have
these at the `SelectWrapper` level._
- onBlur
- onFocus
- onChange


<!--
Thanks for the pull request. Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?

Are there any issues, Slack conversations, Zendesk issues, user stories,
etc. reviewers should consult to understand this pull request better?
For instance:

* Fixes #12345
* See: #67890
-->

## Screenshots/Screencaps

<!-- If you're making a change to the UI, make sure to capture a
screenshot or a short video showing off your work! -->

### Before

<!-- How did the UI you changed look before your changes? Drag your
file(s) below this line: -->

### After

<!-- How does it look now? Drag your file(s) below this line: -->
<img width="1290" alt="Screenshot 2023-10-11 at 11 08 28 AM"
src="https://github.com/MetaMask/metamask-extension/assets/26469696/3a60a891-53e9-49b6-bc2e-7fed8679ee07">


## Manual Testing Steps

<!--
How should reviewers and QA manually test your changes? For instance:

- Go to this screen
- Do this
- Then do this
-->

## Pre-merge author checklist

- [x] I've clearly explained:
  - [x] What problem this PR is solving
  - [x] How this problem was solved
  - [x] How reviewers can test my changes
- [x] Sufficient automated test coverage has been added

## Pre-merge reviewer checklist

- [x] Manual testing (e.g. pull and build branch, run in browser, test
code being changed)
- [x] PR is linked to the appropriate GitHub issue
- [ ] **IF** this PR fixes a bug in the release milestone, add this PR
to the release milestone

If further QA is required (e.g. new feature, complex testing steps,
large refactor), add the `Extension QA Board` label.

In this case, a QA Engineer approval will be be required.

---------

Co-authored-by: George Marshall <george.marshall@consensys.net>
garrettbear added a commit that referenced this issue Dec 7, 2023
## **Description**
Create a `SelectButton` component that supports being used within
SelectWrapper as a triggerComponent or can work as a stand-alone
component.

Component should meet all criteria defined in [insights
report](https://www.figma.com/file/ARJUQovVqX6QkGgiIpPOD3/Select%2FMenu-Audit-and-Insight-Report?type=whiteboard&node-id=0-1&t=4cPcfLkhXCIbGxV5-0)


## **Related issues**

Fixes: #20609 

## **Manual testing steps**

1. Run locally 
2. [Visit
Storybook](http://localhost:6006/?path=/docs/components-componentlibrary-selectbutton--docs)
3. Explore and test stories associated with SelectButton

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
n/a
<!-- [screenshots/recordings] -->

### **After**

Loom cut me off a little early, but covered most of it in there. You can
use the prop shape recommendation in placeholder, defaultValue and
value.


https://www.loom.com/share/53bcab4e40d34ceb893ef0411700ab3d?sid=9d8e7cf3-e27e-4c3a-86c8-418fb34b32a0

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: George Marshall <george.marshall@consensys.net>
@metamaskbot metamaskbot added the release-11.9.0 Issue or pull request that will be included in release 11.9.0 label Dec 7, 2023
dbrans pushed a commit that referenced this issue Dec 11, 2023
## **Description**
Create a `SelectButton` component that supports being used within
SelectWrapper as a triggerComponent or can work as a stand-alone
component.

Component should meet all criteria defined in [insights
report](https://www.figma.com/file/ARJUQovVqX6QkGgiIpPOD3/Select%2FMenu-Audit-and-Insight-Report?type=whiteboard&node-id=0-1&t=4cPcfLkhXCIbGxV5-0)


## **Related issues**

Fixes: #20609 

## **Manual testing steps**

1. Run locally 
2. [Visit
Storybook](http://localhost:6006/?path=/docs/components-componentlibrary-selectbutton--docs)
3. Explore and test stories associated with SelectButton

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
n/a
<!-- [screenshots/recordings] -->

### **After**

Loom cut me off a little early, but covered most of it in there. You can
use the prop shape recommendation in placeholder, defaultValue and
value.


https://www.loom.com/share/53bcab4e40d34ceb893ef0411700ab3d?sid=9d8e7cf3-e27e-4c3a-86c8-418fb34b32a0

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: George Marshall <george.marshall@consensys.net>
@metamaskbot metamaskbot added the release-11.8.0 Issue or pull request that will be included in release 11.8.0 label Jan 2, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-11.8.0 on issue. Adding release label release-11.8.0 on issue, as issue is linked to PR #21897 which has this release label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-11.8.0 Issue or pull request that will be included in release 11.8.0 release-11.9.0 Issue or pull request that will be included in release 11.9.0 team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants