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

feature: Separated Add Resources to checklist #6386

Merged
merged 11 commits into from
Mar 18, 2021

Conversation

GeoffCoxMSFT
Copy link
Member

@GeoffCoxMSFT GeoffCoxMSFT commented Mar 12, 2021

Description

AddResources step of details page used DetailsList which had interaction problems with required resources (not displaying checkboxes) and with readability/layout.

This replaces that step with a checklist style to solve the missing checkbox problem and improve the experience.

Task Item

fixes #6299 (fixed by removing DetailsList)
fixes #6095

Screenshots

image
image

@GeoffCoxMSFT GeoffCoxMSFT self-assigned this Mar 12, 2021
@coveralls
Copy link

coveralls commented Mar 18, 2021

Coverage Status

Coverage remained the same at 52.772% when pulling 2d9d275 on gcox/provision-add-resources into 3afb128 on main.

@GeoffCoxMSFT GeoffCoxMSFT marked this pull request as ready for review March 18, 2021 02:51
@GeoffCoxMSFT GeoffCoxMSFT requested a review from hatpick March 18, 2021 02:51
@GeoffCoxMSFT GeoffCoxMSFT changed the title Separated Add Resources to checklist feature: Separated Add Resources to checklist Mar 18, 2021
hatpick
hatpick previously approved these changes Mar 18, 2021
benbrown
benbrown previously approved these changes Mar 18, 2021
Copy link
Contributor

@benbrown benbrown left a comment

Choose a reason for hiding this comment

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

Code looks awesome.

Small changes requested:

  • Default optional to on. This is the current behavior we should preserve til we can discuss with PMs.
  • Hide the required header if the required list is empty.
  • Verify we didn't regress "edit" scenario where there are no remaining resources available (should jump to JSON view)
  • Allow design to take a look and give final comments

@GeoffCoxMSFT GeoffCoxMSFT dismissed stale reviews from benbrown and hatpick via 65f5aec March 18, 2021 22:38
@GeoffCoxMSFT
Copy link
Member Author

  • Default optional to on. This is the current behavior we should preserve til we can discuss with PMs.

DONE

  • Hide the required header if the required list is empty.

DONE

  • Verify we didn't regress "edit" scenario where there are no remaining resources available (should jump to JSON view)

No change to that code path. Can't get there as my subscription won't successfully deploy all the optional parts.

  • Allow design to take a look and give final comments

UX can inspect the nightlies and give feedback

@GeoffCoxMSFT GeoffCoxMSFT merged commit fc2307f into main Mar 18, 2021
@GeoffCoxMSFT GeoffCoxMSFT deleted the gcox/provision-add-resources branch March 18, 2021 23:19
@cwhitten cwhitten mentioned this pull request May 20, 2021
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* Separated Add Resources to checklist

* Small PR fixes

* Missed on find to includes change

* Handling unmount in async useEffect and timeouts

* Of course useRef

* Updating to check all optional by default

* Don't show section headers when empty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants