Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added possibility to switch off the delay - fixed window behaviour #39

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

thaflo
Copy link

@thaflo thaflo commented Dec 21, 2020

should fix #36
should fix #37
fixes #32

though, resizing behaviour is still not perfect

clicking left mouse button  on a tip shows the next tip
clicking right mouse button on the tip shows the previous tip
added the option to set delay = 0  (though that doesn't work with replicants yet)
@@ -37,7 +37,7 @@ enum
MainWindow::MainWindow()
:
BWindow(BRect(100, 100, 740, 190), B_TRANSLATE_SYSTEM_NAME("Tipster"),
B_TITLED_WINDOW, B_ASYNCHRONOUS_CONTROLS | B_NOT_V_RESIZABLE
B_TITLED_WINDOW, B_ASYNCHRONOUS_CONTROLS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the | B_NOT_V_RESIZABLE? Now the window can be resized in a way that shows a large grey area.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

This way it is possible to have a smaller window but visible the whole tip.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... it's not really smaller, it's narrower but has to be taller. You can always have the whole tip visible by making the window wider. We try to keep the tips as short as possible to fit into a reasonably sized window (or replicated view). IMO keeping the window height equal to the icon button height looks much nicer.

source/MainWindow.cpp Show resolved Hide resolved
source/MainWindow.cpp Show resolved Hide resolved
source/Tipster.cpp Show resolved Hide resolved

BDragger* dragger = new BDragger(this);

BLayoutBuilder::Group<>(this, B_HORIZONTAL, 0)
.SetInsets(-2, -2, 0, -2)
.Add(fIcon)
.SetInsets(1, 1, 1, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those insets were on purpose to avoid the smal spacing around the icon and have it flush with the view borders. It should be kept as is, IMO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that's a question to discuss about :-) For me this way looks more as a button as when it's completely attached

source/Tipster.cpp Show resolved Hide resolved
fRunner = new BMessageRunner(this, message, fDelay);
if (fDelay > 0) {
// if delay is set to Off (0), don't send a runner message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This empty line isn't needed.
More importantly: if you go from some delay to "Off", the previous BMessageRunner is still running and keeps changing tips after the the former delay. So you still have to delete the BMessageRunner if delay == 0. Maybe something like this:

	if (fRunner != NULL) {
		delete fRunner;
		fRunner = NULL;
	}
	if (fDelay > 0) {
	// if delay is set to Off (0), don't send a runner message
		BMessage message(UPDATE_TIP);
		fRunner = new BMessageRunner(this, message, fDelay);
	}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a big help for me! Didn't know how to solve it ...

source/Tipster.cpp Show resolved Hide resolved
return ref;
else if (getLocalTipsFile(ref, BString(language,
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style says "else if" on one line.

BMessage message(UPDATE_TIP);

fMessenger->SendMessage(&message);
if (buttons == 1) //left mouse button
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insted of a comment, you can use the constants B_PRIMARY_MOUSE_BUTTON and B_SECONDARY_MOUSE_BUTTON.

@scottmc
Copy link
Member

scottmc commented Sep 16, 2021

any update on this?

@thaflo
Copy link
Author

thaflo commented Sep 16, 2021

Working on it (again) - thanks for the comment :-)

@humdingerb
Copy link
Member

I cannot comment with a Haiku browser without having to "resolve" that comment thread. Which I can then no longer expand to read...
Anyhoo, I'm still of the opinion a spacing around the icon button looks less clean and tidy than without.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants