Initial implementation of slider widget. #166
Conversation
|
Your implementation looks way different then what see for native windows. Did you use some other native implementation as reference? |
| * | ||
| * Contributors: | ||
| * IBM Corporation - initial API and implementation | ||
| *******************************************************************************/ |
| case SWT.MouseUp -> onMouseUp(event); | ||
| case SWT.MouseHorizontalWheel -> onMouseHorizontalWheel(event); | ||
| case SWT.MouseVerticalWheel -> onMouseVerticalWheel(event); | ||
| case SWT.Paint -> onPaint(event); |
There was a problem hiding this comment.
I'd order the events by importance (most important Paint should come first), but otherwise good to use the light-weight events.
| orientation = style & (SWT.LEFT_TO_RIGHT | SWT.RIGHT_TO_LEFT); | ||
|
|
||
| super.style |= horizontal ? SWT.HORIZONTAL : SWT.VERTICAL; | ||
|
|
There was a problem hiding this comment.
Please remove empty lines after { or before }.
| private void onMouseVerticalWheel(Event event) { | ||
| if (!isVisible()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
IMHO, if not visible, there should not be any event at all. Did you got some while invisible?
|
|
||
| private void onMouseHorizontalWheel(Event event) { | ||
| if (!isVisible()) { | ||
| return; |
| } | ||
|
|
||
| if (event.count > 0) { | ||
| increment(2); |
There was a problem hiding this comment.
What means this hard-coded 2? Why not 1?
| private void onPaint(Event event) { | ||
| if (!isVisible()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
SWT.Paint should not be sent if not visible.
| } | ||
|
|
||
| private void onMouseDown(Event event) { | ||
| // Drag of the thumb. |
There was a problem hiding this comment.
Should it work for any mouse button? If not, please check with if (event.button != 1) return;
|
|
||
|
|
||
| private void onMouseUp(Event event) { | ||
| if (isDragging) { |
There was a problem hiding this comment.
Try with moving the mouse outside the shell and moving it back. There exists a method to get all mouse events (forgot its name).
| } | ||
|
|
||
| private void onMouseMove(Event event) { | ||
|
|
| } else { | ||
| thumbHovered = false; | ||
| redraw(); | ||
| } |
There was a problem hiding this comment.
The if-statement is not necessary - thumbHovered = isThumbHovered; redraw(); should do the same.
| int height = drawingArea.height; | ||
|
|
||
| // Fill background | ||
| if (getBackground() != null) { |
There was a problem hiding this comment.
getBackground() will always return != null.
|
|
||
| // Fill background | ||
| if (getBackground() != null) { | ||
| gc.setBackground(getBackground()); |
There was a problem hiding this comment.
Don't invoke the getter twice but store in local variable.
| public void removeSelectionListener(SelectionListener listener) { | ||
| checkWidget(); | ||
| if (listener == null) | ||
| error(SWT.ERROR_NULL_ARGUMENT); |
There was a problem hiding this comment.
Should be on same line or as block statement ({ ... }).
| * </ul> | ||
| */ | ||
| public int getIncrement() { | ||
| checkWidget(); |
There was a problem hiding this comment.
Probably these checks can be removed for such plain getters. Later, the separate Renderer needs to use these methods and checking for each method is useless wasted CPU time.
| private int minMax(int min, int value, int max) { | ||
| return Math.min(Math.max(min, value), max); | ||
| } | ||
|
|
| this.pageIncrement = pageIncrement; | ||
| } | ||
|
|
||
| private int minMax(int min, int value, int max) { |
There was a problem hiding this comment.
Should be static. Maybe it could be moved to a different utilities class?
No description provided.