search

cappuccino / cappuccino

Web Application Framework in JavaScript and Objective-J
https://cappuccino.dev/
GNU Lesser General Public License v2.1
2.2k stars 333 forks source link

Ensure that the `highlight` property is correctly propagated in CPButton. (Was: Update CPButton. #3004

Open michaelbach opened 2 years ago

michaelbach commented 2 years ago

Ensure that the highlight property is correctly propagated in CPButton. (Was: Update CPButton.j)

cappbot commented 2 years ago

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

daboe01 commented 2 years ago

-#new +AppKit

daboe01 commented 2 years ago

@michaelbach can you please fix the formatting issue (space after colon)?

cappbot commented 2 years ago

Milestone: Someday. Label: AppKit. What's next? A reviewer should examine this issue.

daboe01 commented 2 years ago

+#ready-to-commit

cappbot commented 2 years ago

Milestone: Someday. Labels: #ready-to-commit, AppKit. What's next? The changes for this issue are ready to be committed by a member of the core team.

cacaodev commented 2 years ago

Thank you for your contribution. Before this pr can be merged, we need a better report: what exactly was not working before the pr and what is fixed now. We also need a test (manual in this case) or a path to a test in the repository where the reviewer can verify the change. For more informations about our reporting guidelines: https://www.cappuccino.dev/contribute.html#bug-reports

cappbot commented 2 years ago

Milestone: Someday. Labels: #needs-ui-test, AppKit. What's next? A reviewer should examine this issue.

michaelbach commented 2 years ago

Thank you for your contribution. Before this pr can be merged, we need a better report: what exactly was not working before the pr and what is fixed now. …

Thank you for looking into it.

Before: The getter [someButton isHighlighted] did not return the correct state. In contrast, the local(!) variable _isHighlighted in CPButton works and reports the correct state. Reason: The getter is inherited from the super (CPControl), but since the state is not propagated to super, it cannot report the right state. So either implement an additional getter [… isHighlighted] in CPButton.j, or send the state change to super (that's what my PR does), and super's getter will respond with the correct state.

After: [someButton isHighlighted] returns the correct state.

This problem crops up when one codes a custom button. When pressing (but not releasing the button) the state should be visually indicated, and so the info is needed. The problem can be circumvented by using _isHighlighted, but it's not correct behaviour and the next one will stumble over it.