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

Layout widget in editor is now scrollable #1528

Merged
merged 4 commits into from
Sep 6, 2020
Merged

Conversation

Semphriss
Copy link
Member

Having dozens of tilemaps and other sector elements is no longer a pain to manage - now you can scroll the toolbar and access those sneaky little icons!


Urgh! I can't access all my tilemaps! They're going off-screen!

Screenshot from 2020-09-05 01-19-34

Ah, much better.

Screenshot from 2020-09-05 01-19-47

@serano01
Copy link
Member

serano01 commented Sep 5, 2020

Awesome, especially for the forest worldmap, since it has so many tilesets.

src/editor/layer_icon.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Zwatotem Zwatotem left a comment

Choose a reason for hiding this comment

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

Looks good, but I would block it from further scrolling if there's only one icon left on the screen. Currently you can flush them all to the left.
Btw, I expected mouse scrolling, by the title, maybe I expected too much 😆.

src/editor/object_icon.cpp Outdated Show resolved Hide resolved
@tobbi
Copy link
Member

tobbi commented Sep 5, 2020

Approval from me once the review comments were addressed.

@tobbi
Copy link
Member

tobbi commented Sep 5, 2020

Oh, and check the travis warnings and fix them.

@Semphriss
Copy link
Member Author

I'm gonna fix all of this, I'd just have one question :

Btw, I expected mouse scrolling, by the title, maybe I expected too much.

I'm not sure to understand... It scrolls by hovering the first or last 32 pixels of the widget bar, isn't that the way it should work? I wanted to make it work the same way as the tilesets widget. Do you want me to make it another way? What's the method you originally understood from the title?

@Zwatotem
Copy link
Member

Zwatotem commented Sep 5, 2020

Sorry, my comment didn't make much sense. What I had in mind was mouse wheel scrolling, just like the tile selection panel does, but horizontally.

@Semphriss
Copy link
Member Author

Oh, okay. Should I also make it scrollable with mouse wheel?

@Zwatotem
Copy link
Member

Zwatotem commented Sep 5, 2020

If it isn't much of a deal for you then sure.

@Semphriss
Copy link
Member Author

I'll look into this; I'm pushing my first fix batch in a few moments, I'll use that first batch to see if there are any other Travis errors.

I'll add the mouse wheel scroll (if doable in less than an hour 😆) in the second next commit, along with any other Travis errors if applicable.

@Semphriss
Copy link
Member Author

Alright, I couldn't resist and I did everything in a single commit.

I do have a problem, though; scrolling with the mouse wheel also triggers the editor behind the widget. It seems like my return true; in the mouse wheel function isn't enough to prevent propagation.

@Semphriss
Copy link
Member Author

Nevermind, it's a code 18. It works fine now, and I fixed all the requested changes; I'll commit now.

Copy link
Member

@Zwatotem Zwatotem left a comment

Choose a reason for hiding this comment

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

There is an unintended behaviour in that new feature, the bar keeps scrolling, even when I take the mouse.
See video:
https://youtu.be/BV5S4DYmnbU

src/editor/layers_widget.cpp Outdated Show resolved Hide resolved
src/editor/object_icon.cpp Outdated Show resolved Hide resolved
@tobbi tobbi merged commit 9cd827f into master Sep 6, 2020
@mrkubax10 mrkubax10 deleted the editor_layer_bar_scroll branch August 18, 2023 18:00
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.

4 participants