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

Add student action menu to Student Management page #4961

Merged
merged 3 commits into from
Apr 1, 2022

Conversation

donnapep
Copy link
Collaborator

@donnapep donnapep commented Mar 27, 2022

Changes proposed in this Pull Request

This PR tries using the DropdownMenu component from the @wordpress/components package on the Student Management page. It passes in courseId as a prop, which is logged to the console for demonstration purposes. This would obviously need to change later on to be a userId prop. We can take over this PR when it comes time to hook up the menu.

Note: This PR is not meant to be merged as-is. We can use it to implement the action menu though once the other dependencies are ready.

Testing instructions

  • Execute npm run build.
  • Go to Student Management.
  • Try out the action menu. Currently it does nothing.

Screenshots

Screen Shot 2022-03-27 at 6 49 33 AM

@donnapep donnapep self-assigned this Mar 27, 2022
@donnapep donnapep requested a review from a team March 27, 2022 14:38
@gabrielcaires
Copy link
Contributor

Amazing that we have an example to do it!

What do you think about adding at least 1 test just to make sure that any special requirement is necessary to write tests?

@donnapep
Copy link
Collaborator Author

What do you think about adding at least 1 test just to make sure that any special requirement is necessary to write tests?

Good idea! Will add some at some point this week. 🙂

@donnapep donnapep added this to the 4.4.0 milestone Mar 29, 2022
@donnapep donnapep changed the base branch from trunk to feature/student-management March 30, 2022 11:19
@donnapep
Copy link
Collaborator Author

@gabrielcaires I added a test case. I had to take the console.log statement out and also remove the unused prop, since the linter didn't like it.

Also, when this PR is approved I think I will go ahead and merge it. We can iterate on the implementation when we get to the task to hook up the click events.

Copy link
Member

@m1r0 m1r0 left a comment

Choose a reason for hiding this comment

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

Looks good to me. 🙂

@donnapep donnapep merged commit 4e638b2 into feature/student-management Apr 1, 2022
@donnapep donnapep deleted the try/student-action-menu branch April 1, 2022 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants