Skip to content

Conversation

@michaelchadwick
Copy link
Contributor

@michaelchadwick michaelchadwick commented May 22, 2025

Fixes ilios/ilios#4316

<ToggleYesNo> was only disabled conditionally in one other part of the site, but the component itself has no code to use a @disabled argument, thus this fixes the issue and adds it.

However, what should a disabled toggle look like? And what should the tooltip/message say to warn users when it's disabled?

I made a decision about the disabled-ness of the button, but the tooltip/message is still up for grabs. Feedback welcome!

@netlify
Copy link

netlify bot commented May 22, 2025

Deploy Preview for ilios-frontend ready!

Name Link
🔨 Latest commit da812f8
🔍 Latest deploy log https://app.netlify.com/projects/ilios-frontend/deploys/6830acec03f25f00086dc646
😎 Deploy Preview https://deploy-preview-8585--ilios-frontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@michaelchadwick michaelchadwick force-pushed the frontend-4316-ilm-toggle-disable-if-groups-attached branch from 57bb8ff to 6b2ea2f Compare May 23, 2025 15:37
@michaelchadwick michaelchadwick marked this pull request as ready for review May 23, 2025 17:14
@saschaben
Copy link
Member

Hmm. a couple points. First, this is going to wildly irritate any user who has activated the ILM toggle but needs to flip it back to a regular session. Next, the error message will need to state clearly all the values / attributes that need removal in order to deactivate the toggle. Ideally, the message will also include a "Click here to remove all groups and instructors" type link in order to keep the users from having to navigate to user management and clickety click. That said, the visual toggle is fine, and we can discuss and address the above thoughts at jam.

@michaelchadwick
Copy link
Contributor Author

Partial team discussion conclusion: get full team consensus on potential alternative direction for this fix. Idea brought up: remove toggle entirely, make it a "change this to ILM" button with local restrictions, move "Due prior" to different section since it's not tied to toggle logic.

@michaelchadwick michaelchadwick marked this pull request as draft July 8, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notify Users Instructor and Students Will Be Removed From ILM when Toggle Set to Off

2 participants