Page MenuHome

Fix incorrect cursor position for UI Value Slide (T62939)
ClosedPublic

Authored by Victor Seiji Hariki (seijihariki) on Apr 1 2019, 10:42 PM.

Details

Summary

Fixes incorrect cursor position for number sliders as described on task T62939

The problem was reminiscent from offset logic for the pill-shaped sliders of the previous design.

Still left the else for UI_BTYPE. I don't think it is being used for anything, so in case it's better to remove the else block altogether, I'll remove it.

Diff Detail

Repository
rB Blender

Event Timeline

I can verify that this patch solves the described issue. Not sure if it has any unwanted side effects yet.

source/blender/editors/interface/interface_handlers.c
4794

This variable is only used in one subscope now, makes sense to move the declaration there.

4812

Can you, or @Dalai Felinto (dfelinto), tell me what deler stands for?

source/blender/editors/interface/interface_handlers.c
4812

Dutch variable name meaning "denominator" or "divider". Probably was missed when things were translated to English.

  1. Please for future patches, use arc diff to send the patch (even for small ones). This way the "Context not available" message doesn't show up (aka, we get to see the code around) and it is easier to test the patch.
  2. As a separate commit (before/after) I would say we translate the deler variable as well.

Besides that, the patch seems correct (I haven't tested it though). It makes sense that there was an offset before.
One thing not addressed here, is that 50% won't show in the middle if the property has the animation decorator (e.g., material roughness). It is not the same issue addressed here, but since it is related, I think it was worth mentioning (perhaps it interest the developer to tackle next).

@Jacques Lucke (JacquesLucke) you on charge of the patch? Can you commit it once it is all good?

source/blender/editors/interface/interface_handlers.c
4794

Actually, do you think it's still relevant to keep the offset? I feel that any future implementations to use this class will not be pill-shaped either.

4812

Thanks! I actually didn't know. I will change the name.

Fix Problems noted on differential

  • offs used in only one subscope now local to that scope
  • 'deler' name was not translated from dutch (now 'cursor_x_range')
This revision is now accepted and ready to land.Apr 3 2019, 3:14 PM
This revision was automatically updated to reflect the committed changes.