Hide Access Report banner on teacher homepage #54862
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| /* We are hiding the Census banner to free up space on the Teacher Homepage (November 2023) | ||
| * when we want to show the Census banner again remove the next line | ||
| */ | ||
| showCensusBanner = false; |
There was a problem hiding this comment.
I'm a little surprised it lets you do this. To me, changing the value of a prop (sort of) feels really confusing. What do you think about using a different variable.
Naming is a bit tricky. I'd lean towards forceHideCensusBanner = true but that would require line 80 to change to useState(showCensusBanner && !forceHideCensusBanner), which might be a little confusing. You could instead you a variable like censusBannerAllowed = true (not a big fan of this name, feel free to suggest others) then update line 80 to be useState(showCensusBanner && censusBannerAllowed).
Thoughts?
There was a problem hiding this comment.
Totally agree! Good call, yes I'll update!
bethanyaconnor
approved these changes
Nov 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Undoes the changes made in this PR (which shows the access report banner). Ideally for the future, we could put this behind a DCDO flag to not have to manually add/remove this code when we want to toggle the banner.
Previously showed banner on teacher homepage
No longer shows banner
Links
Jira ticket: here
Previous PR where banner was shown: here
Testing story
Tested locally.