carbon-design-system / ibm-products

A Carbon-powered React component library for IBM Products
https://carbon-for-ibm-products.netlify.app/
Apache License 2.0
87 stars 120 forks source link

Remove #61

Closed carrenelloyd closed 3 years ago

carrenelloyd commented 3 years ago

Catalog # 18

Link to Design

View remove implementation

This may have working code, check with Vikki Paterson.

Maintainer Vikki Paterson

Tasks

Working in experimental package

Continuing in experimental package

Review and promote

lee-chase commented 3 years ago

@vikkipaterson can you comment as to the status of the design it was set to 'status: refining' by @carrenelloyd

lee-chase commented 3 years ago

Still need to add stories and tests

davidmenendez commented 3 years ago

Initial development has been completed for the RemovalModal and all prerequisites listed above have been satisfied. I think this is ready for review.

davidmenendez commented 3 years ago

@vikkipaterson @carrenelloyd this component is ready for a design review

chloepoulterdesign commented 3 years ago

Issue 1

On High impact the modal is a "remove" but the button says "delete". image This is actually the case for a few of them - wherever Remove is used, it should be used in all places, and vice versa with Delete. This also needs to be consistent in the "post delete" modals.

Issue 2

image With the high-impact variations, can we set the field label to be Type [name of resource] to confirm?

Issue 3

There is also no placeholder text in the field (there is in the designs) - but this is not stop-ship.

Issue 4

It's not clear to people that the same component can be repurposed for use in a "delete" action. We should either:

vikkipaterson commented 3 years ago

Issue 1 There shouldn't be an high impact version of remove. High impact is only applicable to delete, whereas remove has the medium or low varieties so the high impact ones should only reference Delete, not remove.

image

Issue 2 Can we change the default text in the high impact modal to be clearer in reflecting the purpose of it. The high impact modal requires the user to manually type in the name of the resource as confirmation to enable the deletion of it. The example text should highlight the consequences of deleting (name of the resource) as well as stating the action can't be undone. The label of the textbox should be instructional as to what they need to type into it to be able to confirm the deletion. e.g. Type [name of resource] to confirm

This is a screen shot of the design in the pattern

image
davidmenendez commented 3 years ago

@chloepoulterdesign @vikkipaterson thanks for the feedback. i'll take care of this ASAP.

davidmenendez commented 3 years ago

@chloepoulterdesign @vikkipaterson upon further inspection of your feedback I think you may have been looking at the IdeRemove component, which is the old component. The new one is called RemovalModal. Some of the feedback provided was still applicable to the new component I worked on, but the screenshot indicates you were looking at the previous component. Here's the new component. The content should be updated as soon as my latest pull request is merged in https://ibm-cloud-cognitive.netlify.app/?path=/story/experimental-removalmodal--without-confirmation

davidmenendez commented 3 years ago

@chloepoulterdesign @vikkipaterson the stories have been updated. please take another look at the with confirmation and without confirmation stories under Experimental/RemovalModal

Levinson1 commented 3 years ago

@chloepoulterdesign can you provide a final review before the end of the week?

chloepoulterdesign commented 3 years ago

Looks good to me. If it is at all possible, it would be great to rename the component to RemoveDeleteModal to make it clear it can be used for either, but otherwise, LGTM!

lee-chase commented 3 years ago

It would appear @davidmenendez that if you rename to RemoveDeleteModal the Chloe is happy to move this into 'Release Review' state.

davidmenendez commented 3 years ago

fantastic. i'll get this done today 👍

sydrosa commented 3 years ago

Review for release

Readiness

Engineering review

Standards

Testing

Documentation