Skip to content

New layer is selected after creation #1328

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 6, 2020
Merged

New layer is selected after creation #1328

merged 3 commits into from
Apr 6, 2020

Conversation

davidlamhauge
Copy link
Contributor

New layer is selected when created. Only tested on Win10, so it should work on all platforms ;-)
closes #1327

@davidlamhauge
Copy link
Contributor Author

Since I don't like the same code repeated in many (four) functions, I now also moved the Q_EMIT to the common function setNewLayerAsSelected().
I'm not sure it's the right decision, and maybe we should omit the common function all together? Two lines repeated in four functions, is not that big a crime...

chchwy
chchwy reviewed Apr 6, 2020
View reviewed changes
core_lib/src/managers/layermanager.cpp
@@ -180,7 +181,7 @@ LayerBitmap* LayerManager::createBitmapLayer(const QString& strLayerName)
const QString& name = nameSuggestLayer(strLayerName);
layer->setName(name);

Q_EMIT layerCountChanged(count());
setNewLayerAsSelected();

Copy link
Member

Choose a reason for hiding this comment

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

Probably not here. load() is called after a Project is loaded. It looks a bit weird to see aything related to NewLayer here.

Copy link
Member

Choose a reason for hiding this comment

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

It's not being called in load(), github is showing a slightly confusing image.. the method is only added in createLayer methods.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, got it. sorry my bad.

MrStevns
MrStevns approved these changes Apr 6, 2020
View reviewed changes
Copy link
Member

@MrStevns MrStevns left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

Alternatively you could have created a getLastLayerIndex() { return count() -1 } and then setCurrentLayer(getLastLayerIndex());

@davidlamhauge
Copy link
Contributor Author

After thinking about it I decided to create a getLastLayerIndex() function, since it may come in handy in other cases.

@MrStevns MrStevns merged commit d694bf0 into pencil2d:master Apr 6, 2020
@MrStevns
Copy link
Member

MrStevns commented Apr 6, 2020

Tested and works as expected, well done 👍

@Jose-Moreno Jose-Moreno added this to the 0.6.5 milestone Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chchwy chchwy chchwy left review comments

@MrStevns MrStevns MrStevns approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.6.5
Development

Successfully merging this pull request may close these issues.

New layers should become automatically selected after creation
4 participants
@davidlamhauge @MrStevns @chchwy @Jose-Moreno