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

fix CPSlider.j #3045

Open michaelbach opened 1 year ago

michaelbach commented 1 year ago

Fix: slider knob did not follow “setObjectValue” when set to “Only stop at tick marks”. Needed a new method closestTickMarkValueToIndex, called in knobRectForBounds. [Tick marks still are not drawn, but binding to value works now.]

cappbot commented 1 year ago

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

daboe01 commented 1 year ago

+#ready-to-commit +Bindings +AppKit

cappbot commented 1 year ago

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

daboe01 commented 4 months ago

i tend not to merge this one. closestTickMarkValueToIndex is not part of cocoa. what is a good use case for this? at least, the method should be underscored to mark it as a private addition. -#ready-to-commit

enquora commented 4 months ago

I'll take a look at this too. We do want to precisely match AppKit APIs (for documentation and reference testing) unless a very good reason exists.

michaelbach commented 4 months ago

Dear Daniel:

i tend not to merge this one. closestTickMarkValueToIndex is not part of cocoa. what is a good use case for this? at least, the method should be underscored to mark it as a private addition. -#ready-to-commit

Please do merge. Use case: without it, one cannot make slider values stop at tick marks and communicate via bindings. I use it very often. Example: https://michaelbach.de/ot/mot-rmi/

closestTickMarkValueToIndex was already called in the code, but was massing, so I added it. Very happy with underscore.

Thanks, Michael -- https://michaelbach.de

enquora commented 4 months ago

I'll look at this next week too. If it introduces a non-cocoa API, I would like to consider carefully. Agree entirely with your rationale, @michaelbach. I also want to introduce more automated and manual tests - we've let this slide a bit over the last two or three years. Restricting ourselves to cocoa-only APIs opens opportunities to automate testing against a native app (not necessarily for this PR, but more complex ones).

cappbot commented 4 months ago

Milestone: Someday. Labels: #new, AppKit, bindings. What's next? A reviewer should examine this issue.

daboe01 commented 4 months ago

closestTickMarkValueToIndex was already called in the code, but was massing, so I added it. Very happy with underscore.

i cannot find any call to closestTickMarkValueToIndex in the current implementation. where did you see this?

daboe01 commented 4 months ago

if this is only used by your code, it may be better to add this to your your codebase by means of a category of CPSlider.