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

feat(Import Step): Move to Create New Step Menu #1445

Conversation

hirokiterashima
Copy link
Member

@hirokiterashima hirokiterashima commented Oct 4, 2023

Notes

  • @breity please feel free to style and re-word elements as you see fit
    • For example, should the "Create New Step" button be "Add New Step" in the project-authoring view?

Changes

  • Move "Import Step" option to "Create New Step" menu (before: "Import Step" was a top-level option in the project-authoring view). This will remove the top-level button in project-authoring view by 1, and make the import step workflow similar to import component flow.

Test

  • "Import Step" option is available through "Add New Step" menu
  • "Import Step" workflow works as before, including back/cancel buttons

Closes #1444

@hirokiterashima hirokiterashima self-assigned this Oct 4, 2023
@hirokiterashima hirokiterashima requested a review from breity October 4, 2023 05:13
@hirokiterashima hirokiterashima marked this pull request as ready for review October 4, 2023 05:13
Copy link
Member

@breity breity left a comment

Choose a reason for hiding this comment

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

Functionality works well. 👍

I made a couple changes:

  • Removed unused description field from NewNodeTemplate.
  • Removed unnecessary aria-label attributes from buttons that have label text.

One request: For consistency, could we add a back button to the "Choose a unit from which to import step(s)" stage in the import sequence?

I think changing "Create New Step" to "Add New Step" could make sense. If we do, we should also change "Create New Lesson" to "Add New Lesson" to be consistent.

@hirokiterashima
Copy link
Member Author

Good suggestions. I made the changes. PTAL.

Copy link
Member

@breity breity left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hirokiterashima hirokiterashima merged commit 2ff3f69 into develop Oct 12, 2023
2 checks passed
@hirokiterashima hirokiterashima deleted the issue-1444-move-import-step-option-to-create-new-step-menu branch October 12, 2023 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Move Import Step option to Create New Step Menu
2 participants