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

Add tooltip to buy and sell buttons. #1412

Merged
merged 1 commit into from Apr 18, 2024

Conversation

Kurtsley
Copy link
Contributor

Fixes #1410.

Adds tooltip text to the buy and sell buttons. Switches if the "MarketOnRight" option is toggled. Adds "Transfer" to transfer screen buttons and "Destroy" and "Contain" to alien containment screen.

@XCOM-HUB
Copy link

Despite having installed your fix, I still can't see the Buy and Sell tooltip when hovering the cursor on the Buy and Sell buttons.

@Kurtsley
Copy link
Contributor Author

Strange, have you tried the alien containment and transfer screens? Try toggling the MarketOnRight option.

@XCOM-HUB
Copy link

I have tried toggling the MarketOnRight option without any results.

@Kurtsley
Copy link
Contributor Author

I'll check it out on a different system later and see if I can reproduce what you see.

@Kurtsley
Copy link
Contributor Author

I just tested on my Windows and Linux machines and I saw the buy, sell, and transfer tooltips pop up (didn't test alien containment). I have no idea why you aren't seeing them. I guess we can wait until others test this and see what happens.

@XCOM-HUB
Copy link

XCOM-HUB commented Apr 17, 2024

The Buy/Sell tooltip pops up when you are in the first menu after clicking the Base icon in the Cityscape, it's just the slider inside the Buy/Sell Market that doesn't show a tooltip if you hover the mouse cursor over the left or right arrows. It doesn't matter if you toggle the MarketOnRight option. It still persists for me on the latest version from AppVeyor 0.0.1031

I wonder if I have to do a clean install for it to work, but that shouldn't be the problem. I can see the other fixes working so I should also be seeing this fix work as well.

This is how it should look like:
Skärmbild 2024-04-14 210049

And "Sell"
Skärmbild 2024-04-17 155436

@Kurtsley
Copy link
Contributor Author

That is exactly what I am seeing. How are you installing this branch to test these changes?

@XCOM-HUB
Copy link

I just install the latest version from AppVeyor. I have tested with a clean install and I have installed version 0.0.1031

@Kurtsley
Copy link
Contributor Author

Okay, that makes sense then. This fix hasn't been merged yet and isn't in version 0.0.1031.

@XCOM-HUB
Copy link

Okay! How can I apply this fix then?

@Kurtsley
Copy link
Contributor Author

Actually, I'm wrong. I see it in the list of PRs. I'll try testing from AppVeyor instead of my usual method and see what happens.

@Kurtsley
Copy link
Contributor Author

I guess I don't know how AppVeyor works because this PR definitely hasn't been merged into master. I see other PRs that haven't been merged yet either. I guess you could grab my add-transaction-tooltip branch from my fork and build that? That would work but it is a pain.

@XCOM-HUB
Copy link

I haven't done that before. Can anyone merge the fixes that haven't yet been merged into the master? I'm only used to installing every fix from AppVeyor.

@Kurtsley
Copy link
Contributor Author

I got it, if you select the specific build that the fix was introduced it works. This is the tooltip build:
https://ci.appveyor.com/project/OpenApoc/openapoc/builds/49614024

@XCOM-HUB
Copy link

It works, but whenever I install a newer version from AppVeyor it just disappears again. When will this fix be merged with the latest version from AppVeyor?

@Kurtsley
Copy link
Contributor Author

Probably within a few days depending on people's schedules. This is a pretty low priority fix.

@FilmBoy84
Copy link
Collaborator

Apologies for the delay in review, to further what kurtsley says, you can always check out a PR specific build before it's merged to master by checking the following @XCOM-HUB

Look at the pull details
image
image

On Appveyor find the most recent build referencing that PR (If not merged)
image
image

Thanks for the fix Kurtsley 🍻

@FilmBoy84 FilmBoy84 merged commit 99b00d4 into OpenApoc:master Apr 18, 2024
3 checks passed
@XCOM-HUB
Copy link

Ok! Hopefully Kurtsley's fix will be forever merged with all the latest fixes. No fix left behind! 👍

@XCOM-HUB
Copy link

Great news! This fix seems to be a permanent one as of 0.0.1035, and it also works in the latest 0.0.1037 as well.

I'm loving this 💗

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

Successfully merging this pull request may close these issues.

Hovering mouse cursor over Buy/Sell button does not display a tooltip prompt
3 participants