issue-#98 improvements of GUI controls

[Josef Templ]

Work on this issue can only start after issue-#94 is merged.

- Josef

[Ivan Denisov]

Josef, does the improvements mainly consist of increasing the width of label elements. Or there are some other improvements?

[Josef Templ]

For the issue see http://redmine.blackboxframework.org/issues/98.

For the changes see http://redmine.blackboxframework.org/pr ... 73062d3201.

The changes are as proposed by Koen Dessenger in CPC 1.7 rc6
with the following exceptions:

HostCFrames.Init is not changed because this change is not used, as far as I see.

HostCmds.PasteObjectGuard is unchanged because I could not figure out which problem it solves.

Code: Select all

	PROCEDURE PasteObjectGuard* (VAR par: Dialog.Par);
		VAR ops: Controllers.PollOpsMsg;
	BEGIN
		Controllers.PollOps(ops);
		IF ~(Controllers.paste IN ops.valid)
			OR ~HostClipboard.ConvertibleTo("")
			(*>>>change start*) OR (Controllers.FocusView() IS Controls.Control) (*<<<change end*) THEN par.disabled := TRUE END
	END PasteObjectGuard;

HostMenus.PopupMenu is unchanged because I could not figure out which problem it solves.
It would imply a significant change of the import order, so it should have a very good reason.

The change in HostCFrames.HandleCommand for UpDownField is not clear to me.
It introduces a check for invalid input but such a check is only done for UpDownField.

In general, I must admit that I do not understand all the details of the proposed changes.
However, it seems that the author (Koen Dessenger) is a real Windows GUI expert
and therefore it is likely that the changes work well.

Please review carefully and let me know if any of the missing changes need to be included.

- Josef

[Robert]

There is a current restriction that UpDown Controls are not compatible with LONGINTs.

Is it worth lifting this restriction?

[Josef Templ]

Shifting the decimal order by pressing Ctrl is now supported.
It also works with the mouse wheel.

see changes at http://redmine.blackboxframework.org/pr ... 2c777b13ca.

Supporting LONGINT is definitely not needed.

- Josef

[Robert]

Josef - Possibly I have made an error copying the changes, but ...

The multiply by 10 logic works for positive numbers (tested with INTEGERs & BYTEs).

1 - However it does not work correctly with negative numbers. Overflow should limit at MIN (TYPE), but for INTEGERs it can overflow to a positive number, while for BYTEs is saturates at the wrong negative value.

(Note: With my code I did not think about BYTEs or SHORTINTs, so that is probably wrong also!)

2 - Is there a reason for having "f.max / 10" rather than "f.max DIV 10"?

3 - We should add to the Controls Docu:

Code: Select all

UpDownField
The UpDownField is a special text field linked to a BYTE, SHORTINT, INTEGER or LONGINT variable. The value of the integer can be incremented and decremented through arrows, or by using the mouse wheel. If the <Ctrl> key is depressed the value is multiplied or divided by 10 rather than being incremented or decremented by 1.

Supporting LONGINT is definitely not needed.

Asd you can see from the previous paragraph the Docu explicitly says that LONGINTs should be supported!
I don't personally care, but orthogonality is often a desirable property.

[Robert]

How about ?

Code: Select all

	PROCEDURE (f: UpDownField) KeyDown (ch: CHAR);
		VAR val: INTEGER;
	BEGIN
		ASSERT(~f.disabled, 100);
		IF (ch = AU) OR (ch = AD) THEN
			val := f.val;
			IF WinApi.GetKeyState(WinApi.VK_CONTROL) < 0 THEN (* Ctrl key pressed *)
				IF ch = AU THEN
					IF val > 0 THEN
						IF val <= f.max DIV 10 THEN val := val * 10 ELSE val := f.max END
					ELSIF val < 0 THEN
						IF val >= -((-1-f.min) DIV 10) THEN val := val * 10 ELSE val := f.min END
					END
				ELSE
					IF val >= 0 THEN val := val DIV 10 ELSE val := (val + 9) DIV 10 END
				END
			ELSE
				IF ch = AU THEN
					IF val <= f.max - f.inc THEN val := val + f.inc ELSE val := f.max END
				ELSE
					IF val >= f.min + f.inc THEN val := val - f.inc ELSE val := f.min END
				END
			END;
			IF val # f.val THEN f.Set(f, val); f.Update END
		ELSIF (ch # 0DX) THEN
			SendKey(ch, f.i.ctrl)
		END
	END KeyDown;

Note that I think 10 × 0 = 0.

[Robert]

In the above I prefer to replace

Code: Select all

IF val >= -((-1-f.min) DIV 10) THEN

by

Code: Select all

IF val >= ((f.min + 9) DIV 10) THEN

as it is simpler to explain, and, I think, the former could fail if someone were to invent a type with f.min a multiple of 10!

To be honest I haven't tested this last change, but will when I get home this evening.

[Josef Templ]

fixed and docu updated.
See http://redmine.blackboxframework.org/pr ... e4bcdf9a67.

It is also simplified in the sense that saturation is not at min/max but when val * 10 exceeds min/max.
So with a BYTE, for example, you get 0, 1, 10, 100, and then it stops.
The same with negative numbers: -1, -10, -100.

This semantics is easier to describe and more intuitive, I think.
When val * 10 is no longer possible, it is no longer done.
Jumping to min/max destroys the number and the semantics.

- Josef

[Robert]

Still wrong I think. The line

Code: Select all

IF val >= f.min DIV 10 THEN val := val * 10 END

should be, as I said earlier

Code: Select all

IF val >= (f.min + 9) DIV 10 THEN val := val * 10 END

Try multiplying -214748365 by 10.

I don't think multiplying 0 by 10 and getting 1 is "simpler and more intuitive" than getting 0. But if we stay with it we need to add to the Docu

The value of the integer can be incremented and decremented through arrows, or by using the mouse wheel. If the <Ctrl> key is depressed the value is multiplied or divided by 10 rather than being incremented or decremented by 1 except that attempts to multiply 0 by 10 lead to 1. Multiplications that would lead to overflow are inhibited.

I still prefer such multiplications to clip at MIN or MAX.

To take BYTE as an example. If the current value is 15 and I insert a 0 it becomes 150, then subsequently clips at 127. I guess I expect that multiplying 15 by 10 using the <Ctrl> key should also clip at 127.

This behaviour is BBox 1.6. My other installation is CPC edition 12.12.2013. Its behaviour is quite different, and more like what you are suggesting, ie totally inhibiting potential overflows. So I guess what behaviour you expect here is conditioned by the behaviour you are used to.