Skip to content

Fixed Issue #1942 [BUG] Some modifying actions can still be performed#1989

Open
Jouvens-Corantin wants to merge 1 commit into
pencil2d:masterfrom
Jouvens-Corantin:master
Open

Fixed Issue #1942 [BUG] Some modifying actions can still be performed#1989
Jouvens-Corantin wants to merge 1 commit into
pencil2d:masterfrom
Jouvens-Corantin:master

Conversation

@Jouvens-Corantin

Copy link
Copy Markdown

Implemented validation checks for hidden layers across the Clear Frame, Select All, Paste, Paste from Previous Frame, and Move Key operations. Additionally, added an automatic deselect function when switching to a hidden layer to prevent selection artifacts.

@chchwy chchwy changed the title Fixed Issue #1942 ([BUG] Some modifying actions can still be performe… Fixed Issue #1942 [BUG] Some modifying actions can still be performed Feb 21, 2026
@chchwy chchwy self-assigned this Feb 21, 2026

@MrStevns MrStevns left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution 🙂

There are of course still room for improvement in this area, for example we could check in Layer::moveSelectedFrames and Layer::reverseOrderOfSelection, Layer::setFrameSelected, Layer::toggleFrameSelected

But some validation is better than no validation.

@github-project-automation github-project-automation Bot moved this from Needs Review to Approved in Pull Request Priority Feb 23, 2026
{
if (!visible()) {
return false;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer to keep this logic out of the Layer class.

Restricting frame movement based on visibility is a UI-level behavior designed to prevent accidental edits. however, from a data perspective, the model should allow moving frames regardless of visibility. We should probably move this check to the Interaction or Controller layer.

@MrStevns MrStevns Feb 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that's up for debate. Imo. This is exactly where the logic should be, rather than in the UI. If we decide to build two UI clients, then the logic should be handled by our core rather than each respective client?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we plan to add script system or API. I believe we did discuss the feasibility before.

…performed on hidden layers)

Added an if statements to check for the hidden layer property throughout the clear frame, select all, paste, paste from pervious frame, and move key operations. Added a deselect all function when switching from a unhidden to hidden layer to prevent selection issues.
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

3 participants