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

Fix: Readd toolTipContainer styles to sections-refresh #58696

Merged
merged 1 commit into from
May 21, 2024

Conversation

ebeastlake
Copy link
Contributor

@ebeastlake ebeastlake commented May 16, 2024

When PR https://github.com/code-dot-org/code-dot-org/pull/58038/files was merged, the sections-refresh module was imported into the SectionAccessToggle.tsx and had unintended consequences in the teacher dashboard because it has some very broad CSS selectors.

The Slack thread on the original issue was: https://codedotorg.slack.com/archives/C045UAX4WKH/p1715806578458379?thread_ts=1715803641.843369&cid=C045UAX4WKH

When I put up the revert of the revert, I moved the CSS for the tooltips into a dedicated access-controls.module.scss, which kept the overly scoped sections-refresh rules from applying across the teacher dashboard: https://github.com/code-dot-org/code-dot-org/pull/58659/files/a681e72105862d5b78068298fe4bbd66267b5f46..67852a06652225a93a2c4e5e47a2bb54ca788335

BUT, I made a mistake by removing the toolTipContainer from the sections-refresh container https://github.com/code-dot-org/code-dot-org/pull/58659/files/a681e72105862d5b78068298fe4bbd66267b5f46..67852a06652225a93a2c4e5e47a2bb54ca788335#diff-e26532ee34fdf045c225c00b1fd13d15158929aba3b198ee9711b5e1f994806dL182 because I assumed it had been added by @kakiha11 in the original PR.

It is necessary for the other Advanced Settings tooltips, and wasn't caught by existing UI tests.

Links

Testing story

I confirmed locally that the appearance and CSS being applied here now match what they were before yesterday night.


Screenshot 2024-05-16 at 2 06 35 PM
Screenshot 2024-05-16 at 2 06 42 PM

Deployment strategy

Follow-up work

Added a task to add UI tests for Advanced Settings section toggles: https://codedotorg.atlassian.net/browse/TEACH-1113

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@ebeastlake ebeastlake requested review from stephenliang, bethanyaconnor, kakiha11, a team and lfryemason and removed request for a team May 16, 2024 20:30
Copy link
Contributor

@bethanyaconnor bethanyaconnor left a comment

Choose a reason for hiding this comment

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

Would love for teacher tools to take a look but this looks simple enough!

@ebeastlake ebeastlake merged commit 0952716 into staging May 21, 2024
2 checks passed
@ebeastlake ebeastlake deleted the fix/advanced-settings-ui branch May 21, 2024 17:03
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.

None yet

4 participants