-
Notifications
You must be signed in to change notification settings - Fork 56
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
[ENG-5879] Hide Categories Section on Registrations #2352
[ENG-5879] Hide Categories Section on Registrations #2352
Conversation
bea1380
to
060157a
Compare
Pull Request Test Coverage Report for Build 11296277310Details
💛 - Coveralls |
060157a
to
1fba6b6
Compare
There was a problem hiding this 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question.
@tracked hideCategories = true; | ||
// Private properties | ||
expandCitations = false; | ||
|
||
@task({ on: 'didReceiveAttrs'}) | ||
@waitFor | ||
async fetchProvider() { | ||
this.provider = await this.registration?.provider; | ||
this.hideCategories = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need hideCategories to be tracked and set it in fetchProvider, or could you leave it untracked and only set it at declaration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brianjgeiger I made hideCategories tracked in case we needed it to be dynamic in the future, but since it’s always true right now, I could remove it. Should we make it untracked to keep things simple?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's keep it simple for now. I have a feeling it's more likely that we'll get rid of categories entirely rather than make it dependent on something else.
1fba6b6
to
49a60e2
Compare
49a60e2
to
e7aa290
Compare
Purpose
Hide Categories Section on Registrations
Summary of Changes