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.

Sign up for GitHub

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

Jump to bottom

New: Add option for disabled button text (fixes #207) #214

Merged
merged 9 commits into from
Mar 18, 2024
Merged

New: Add option for disabled button text (fixes #207) #214

merged 9 commits into from
Mar 18, 2024

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Mar 7, 2024 •
edited
Loading

Fix #207

New

  • Adds the ability to show different text when the button is disabled. The new properties are disabledText and disabledAriaLabel.

Testing

  1. Set up Trickle using the code from example.json
  2. _styleBeforeCompletion should be set to visible so that you can see the button before it is enabled
  3. Test different scenarios including:
  • text is set but disabledText is not set.
  • text and disabledText are set as well as startText and finalText

The order of precedence that text will be shown is:

disabledText
startText
finalText
text

For instance, if the Trickle is on the final block and disabledText is set, that will be used. Otherwise, it will check for finalText when the button is disabled. Finally, it will fall back to text.

@swashbuck swashbuck self-assigned this Mar 7, 2024
@joe-allen-89
Copy link
Contributor

It's working as described, it feels a bit strange that throughout the page trickle says it's locked but on the last block it doesn't if finalText is set despite it being locked, might be confusing to users.

@swashbuck
Copy link
Contributor Author

It's working as described, it feels a bit strange that throughout the page trickle says it's locked but on the last block it doesn't if finalText is set despite it being locked, might be confusing to users.

@joe-allen-89 Maybe we should add locked text for final and start as well? We never use those options in projects, as far as I know.

@oliverfoster
Copy link
Member

I'd prefer you called it disabledText and disabledAriaLabel. As locking is a model state and disabled is a button state. The convention is to put the descriptor first - playText, stopText, itemText, resumeButtonText, promptText, buttonText, startText, finalText, backLabel, nextLabel etc.

@oliverfoster
Copy link
Member

oliverfoster commented Mar 8, 2024 •
edited
Loading

It's working as described, it feels a bit strange that throughout the page trickle says it's locked but on the last block it doesn't if finalText is set despite it being locked, might be confusing to users.

@joe-allen-89 Maybe we should add locked text for final and start as well? We never use those options in projects, as far as I know.

Maybe it should just take precedence over startText and finalText?

disabledText
finalText
startText
text

@swashbuck swashbuck changed the title New: Add option for locked button text (fixes #207) New: Add option for disabled button text (fixes #207) Mar 8, 2024
@swashbuck
Copy link
Contributor Author

@joe-allen-89 @oliverfoster Changes made and PR description updated

oliverfoster
oliverfoster approved these changes Mar 9, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
joe-allen-89
joe-allen-89 approved these changes Mar 11, 2024
View reviewed changes
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.

👍

@kirsty-hames kirsty-hames mentioned this pull request Mar 12, 2024
Open
@swashbuck swashbuck mentioned this pull request Mar 18, 2024
Merged
cahirodoherty-learningpool
cahirodoherty-learningpool approved these changes Mar 18, 2024
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@swashbuck swashbuck merged commit 6512957 into master Mar 18, 2024
@swashbuck swashbuck deleted the issue/207 branch March 18, 2024 21:13
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 7.2.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
Reviewers

@joe-allen-89 joe-allen-89 joe-allen-89 approved these changes

@cahirodoherty-learningpool cahirodoherty-learningpool cahirodoherty-learningpool approved these changes

@oliverfoster oliverfoster oliverfoster approved these changes

Assignees

@swashbuck swashbuck

Labels
enhancement released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Alternate button text when locked
4 participants
@swashbuck @joe-allen-89 @oliverfoster @cahirodoherty-learningpool