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: display padlock for locked items (fixes #209) #210

Merged
merged 4 commits into from
Mar 21, 2024
Merged

Conversation

kirsty-hames
Copy link
Contributor

Fixes #209

Update

As per adaptlearning/adapt-contrib-vanilla#446, locked buttons should display a padlock icon.

I considered whether the padlock icon should replace the progress indicator when locked but for now decided to keep this as supplementary.

locked_plp_buttons

Note, the addition of `.pagelevelprogress__item-icon` could be later used to replace progress bars with icons as per #181.

Copy link
Contributor

@joe-allen-89 joe-allen-89 left a comment

Choose a reason for hiding this comment

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

👍

@swashbuck
Copy link
Contributor

I considered whether the padlock icon should replace the progress indicator when locked but for now decided to keep this as supplementary.

@kirsty-hames I prefer only showing the lock icon instead of both the lock icon and (nearly) empty progress bar.

  1. The progress bar will always essentially be at zero percent for locked items, so it does not serve much purpose.
  2. It is cleaner with only the locked icon or the progress bar (not both). Having both also reduces the amount of space you have for the title before the text wraps.

The only drawback is that you won't see "optional content" unless the item has been unlocked, but not sure how important that is? Curious if others agree.

@kirsty-hames
Copy link
Contributor Author

kirsty-hames commented Mar 19, 2024

I considered whether the padlock icon should replace the progress indicator when locked but for now decided to keep this as supplementary.

@kirsty-hames I prefer only showing the lock icon instead of both the lock icon and (nearly) empty progress bar.

  1. The progress bar will always essentially be at zero percent for locked items, so it does not serve much purpose.
  2. It is cleaner with only the locked icon or the progress bar (not both). Having both also reduces the amount of space you have for the title before the text wraps.

Thanks @swashbuck. I agree with your points and this would also be my preference.

The only drawback is that you won't see "optional content" unless the item has been unlocked, but not sure how important that is? Curious if others agree.

.pagelevelprogress__item-optional is rendered separately to .pagelevelprogress__indicator so it's possible to display both the lock icon and "optional content". See screenshot below. Changes committed in e160cd5.

hide_progress_when_locked

- render icon after 'optional content' so icons display vertically aligned.
-  justify btn content.
- amend indicator margin for alignment with icons.
@kirsty-hames kirsty-hames requested a review from swashbuck March 19, 2024 11:04
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.

👍 Looks and works great!

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 cee2782 into master Mar 21, 2024
@oliverfoster oliverfoster deleted the issue/209 branch March 21, 2024 10:44
github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
# [7.4.0](v7.3.1...v7.4.0) (2024-03-21)

### Update

* display padlock for locked items (fixes #209) (#210) ([cee2782](cee2782)), closes [#209](#209) [#210](#210)

### Upgrade

* Bump ip from 1.1.8 to 1.1.9 (#206) ([3575a4e](3575a4e)), closes [#206](#206)
Copy link

🎉 This PR is included in version 7.4.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 for locked items
5 participants