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

Replace ember render modifiers in learner group root component #8281

Conversation

stopfstedt
Copy link
Member

@stopfstedt stopfstedt commented Dec 20, 2024

@stopfstedt stopfstedt force-pushed the replace_ember_render_modifiers_in_learner_group_components branch from 9a68207 to 33af894 Compare December 20, 2024 01:05
@stopfstedt stopfstedt changed the title Replace ember render modifiers in learner group components Replace ember render modifiers in learner group root component Dec 20, 2024
@stopfstedt stopfstedt force-pushed the replace_ember_render_modifiers_in_learner_group_components branch from e340ad3 to bd99907 Compare December 20, 2024 18:52
@stopfstedt stopfstedt marked this pull request as ready for review December 20, 2024 18:52
Copy link
Member

@jrjohnson jrjohnson left a comment

Choose a reason for hiding this comment

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

One, if you feel like it, change. Marking as approved though. In case you don't.

{{t "general.uploadGroupAssignments"}}
{{else}}
{{t "general.members"}}
({{this.usersForUserManager.length}})
Copy link
Member

Choose a reason for hiding this comment

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

Surprised at how much more readable this is.

this.url = this.args.learnerGroup.url;
}

@cached
Copy link
Member

Choose a reason for hiding this comment

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

Nothing async here, doesn't need to be @cached

Copy link
Member Author

Choose a reason for hiding this comment

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

corrected ✔️

}

get learnerGroupTitle() {
return this.args.learnerGroup.title;
Copy link
Member

Choose a reason for hiding this comment

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

curious why this is a getter, but location and url are both set in the constructor?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, never mind. Those can be changed. Got it.

@dartajax dartajax added the run ui tests Run the expensive UI tests label Dec 20, 2024
@dartajax dartajax merged commit 8f03a49 into ilios:master Dec 21, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run ui tests Run the expensive UI tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants