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

Update: padlock replaces icon when trickle button locked (#217) #218

Merged
merged 2 commits into from Mar 21, 2024

Conversation

kirsty-hames
Copy link
Contributor

Fixes #217

Update

As per Vanilla issue #446, locked disabled buttons should display a padlock icon.

Testing

Enable the following on _trickle._button:

        "_styleBeforeCompletion": "visible",
        "_hasIcon": true

Trickle button should display a padlock icon whilst locked.

trickle_btn_locked

On completing content, trickle button should unlock and the standard chevron icon displays.

trickle_btn_unlocked

@swashbuck
Copy link
Contributor

swashbuck commented Mar 18, 2024

@kirsty-hames I do not actually see the is-locked class on the button when it's disabled. updateButtonState(), which adds the is-locked class, doesn't seem to be called until the content has been completed. I think either we should keep the class on the button in the template where it was removed or else call updateButtonState() prior to the content being completed?

Also, to align with another Trickle PR, could we call the class is-disabled instead of is-locked?

I'd prefer you called it disabledText and disabledAriaLabel. As locking is a model state and disabled is a button state.

@kirsty-hames
Copy link
Contributor Author

@kirsty-hames I do not actually see the is-locked class on the button when it's disabled. updateButtonState(), which adds the is-locked class, doesn't seem to be called until the content has been completed. I think either we should keep the class on the button in the template where it was removed or else call updateButtonState() prior to the content being completed?

Hey @swashbuck, both .is-disabled and .is-locked classes should be appended to the button when locked. See screenshot below.
To test this, I've pulled down master (since PR #214 has been merged) and added in the less/trickle-button.less amend from this PR. Enabled the following on _trickle._button.

        "_styleBeforeCompletion": "visible",
        "_hasIcon": true

Could you re-test this please?

is-locked

Also, to align with another Trickle PR, could we call the class is-disabled instead of is-locked?

I'd prefer you called it disabledText and disabledAriaLabel. As locking is a model state and disabled is a button state.

Agreed, disabled is the button state. .is-disabled is appended to all disabled buttons and .is-locked is an additional class to distinguish buttons that are temporarily disabled due to step locked content. This was raised as a global issue, Allow for locked and inactive disabled buttons to be visually different, and is used for styling only. Seeing as .is-locked is used as a bolt on to .is-disabled rather than instead of, are you happy for the two classes to remain?

@kirsty-hames
Copy link
Contributor Author

@kirsty-hames I do not actually see the is-locked class on the button when it's disabled. updateButtonState(), which adds the is-locked class, doesn't seem to be called until the content has been completed. I think either we should keep the class on the button in the template where it was removed or else call updateButtonState() prior to the content being completed?

Update: I can replicate this on blocks not in an assessment. As per @swashbucks suggestion, I've reintroduced the button template .is-disabled class and introduced .is-locked class in 325a04c. This works as expected for both assessment and non-assessment blocks.

Copy link
Contributor

@swashbuck swashbuck left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oliverfoster oliverfoster merged commit 3a53d29 into master Mar 21, 2024
@oliverfoster oliverfoster deleted the issue/217 branch March 21, 2024 10:19
github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
# [7.3.0](v7.2.0...v7.3.0) (2024-03-21)

### Update

* padlock replaces icon when trickle button locked (#217) (#218) ([3a53d29](3a53d29)), closes [#217](#217) [#218](#218)
Copy link

🎉 This PR is included in version 7.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

Display padlock when Trickle button locked
4 participants