Improve icon logic #9
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Removed unused UserInputService import. Signed-off-by: Ryan Luu <ryan.luu@ryanluu.dev>
Signed-off-by: Ryan Luu <ryan.luu@ryanluu.dev>
Signed-off-by: Ryan Luu <ryan.luu@ryanluu.dev>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the topbar icon logic to use an event-driven approach for synchronizing the icon's visual state with the inventory's open/close state, and improves the API documentation with practical code examples.
Key changes:
- Replaced manual input handling with
BackpackScript.StateChangedevent subscription to automatically sync icon state with inventory visibility - Modified icon toggle handler to only call
BackpackScript.OpenClose()when triggered by user interaction (viafromSourceparameter), preventing redundant toggles - Added descriptive code samples in the documentation demonstrating usage of
OpenClose()andStateChangedAPIs
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/TopbarIcon.client.luau | Refactored event handling to use BackpackScript.StateChanged for icon state synchronization and renamed image constants to uppercase |
| docs/api-reference.md | Added introductory text and code samples demonstrating OpenClose() and StateChanged usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ryanlua
added
bug
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ryan Luu <ryan.luu@ryanluu.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request updates the documentation and refactors the
TopbarIcon.client.luauscript to improve clarity and functionality for the CoreGui backpack inventory toggle and state handling. The main changes include clearer sample code in the API docs and a more robust event-driven approach in the icon script.Documentation improvements:
OpenClose()from a button'sActivatedevent.StateChangedevent to detect when the inventory is opened or closed.Code refactoring and event handling:
TopbarIcon.client.luauto uppercase for clarity and consistency.BackpackScript.StateChanged.Eventto update the icon's selected state based on inventory open/close events.BackpackScript.OpenClose()when the toggle is initiated by the user, improving separation of concerns and preventing redundant toggles.