Differential D20430

Enable/disable sub-styles on Bevel and Emboss toggle in Layer Styles (WIP)
ClosedPublic
Actions

Authored by gernene on Apr 10 2019, 12:15 PM.

Details

Reviewers
rempt
Group Reviewers
VDG
Krita
Summary

UI currently does not make it clear that "Contour" and "Texture" layer styles are only applied to canvas when "Bevel and Emboss" is active.

Changes in commit:
Disable "Contour" and "Texture" by default and re-enables them when "Bevel and Emboss" is applied.
Add code comments to clarify feature for other devs

WIP: Layer Style panel must be closed and re-opened for sub-style toggle to be visible/take effect.

BUG: 396015

Test Plan

Toggle Bevel and Emboss in Layer Styles panel

Diff Detail

Repository
R37 Krita
Branch
layer-styles-toggle
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10695
Build 10713: arc lint + arc unit
gernene created this revision.Apr 10 2019, 12:15 PM
Restricted Application added a reviewer: Krita. · View Herald TranscriptApr 10 2019, 12:15 PM
Restricted Application added a project: Krita. · View Herald Transcript
gernene requested review of this revision.Apr 10 2019, 12:15 PM
tusooaw added a subscriber: tusooaw.Apr 10 2019, 12:29 PM
tusooaw added inline comments.
libs/ui/dialogs/kis_dlg_layer_style.cpp
208

Using indices may make the code hard to maintain. Can you set names of items in QtDesigner? If it is possible, it would be better to use names instead of indices.

libs/ui/dialogs/kis_dlg_layer_style.h
254

slotBevelAndEmbossChanged(bool) might be a more intuitive signature since we are not really notifying someone else about the change.

rempt added a subscriber: rempt.Apr 10 2019, 1:08 PM
rempt added inline comments.
libs/ui/dialogs/kis_dlg_layer_style.cpp
115

connect connect(wdgLayerStyles.lstStyleSelector, SIGNAL(itemClicked(QListWidgetItem*)), SLOT(notifyBevelAndEmbossToggle(QListWidgetItem*)));

instead.

206

Better use if (wdgLayerStyles.lstStyleSelector->item(6)->checkState() == Qt::Checked) {

gernene updated this revision to Diff 55901.Apr 10 2019, 2:23 PM

Enable/disable sub-styles on Bevel and Emboss toggle in Layer Styles

gernene added a comment.Apr 10 2019, 2:25 PM

Update: I've implemented all the revisions with the exception of naming the list items instead of using indices as I can't seem to name them in Designer.

rempt accepted this revision.Apr 15 2019, 1:20 PM
This revision is now accepted and ready to land.Apr 15 2019, 1:20 PM
woltherav added a subscriber: woltherav.Apr 30 2019, 2:26 PM

did this ever get merged? I think so, right?

Revision Contents
Changeset List

Diff 55901

View Options

libs/ui/dialogs/kis_dlg_layer_style.h

Loading...
View Options

libs/ui/dialogs/kis_dlg_layer_style.cpp

Loading...
View Options

libs/ui/layerstyles/wdglayerstyles.ui

Loading...