Fixed color selection for map sketching #4274
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||
| import QtQuick | ||||||
| import QtQuick.Controls | ||||||
|
|
||||||
| ScrollView { | ||||||
| id: root | ||||||
|
|
||||||
| required property var colors | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
it would be nice to specify fully what we want there |
||||||
| required property var controller | ||||||
|
Contributor
There was a problem hiding this comment. I wouldn't couple controller tightly with this component. Using signals is preferred here |
||||||
| required property bool showEraseButton | ||||||
|
|
||||||
| height: scrollRow.height | ||||||
| ScrollBar.vertical.policy: ScrollBar.AlwaysOff | ||||||
| ScrollBar.horizontal.policy: ScrollBar.AlwaysOff | ||||||
|
|
||||||
| Row { | ||||||
| id: scrollRow | ||||||
| spacing: __style.margin12 | ||||||
| padding: __style.margin4 | ||||||
| anchors.centerIn: parent | ||||||
|
|
||||||
| Repeater { | ||||||
| model: colors ?? null | ||||||
|
Contributor
There was a problem hiding this comment. will be always specified and never null |
||||||
|
|
||||||
| MMRoundButton { | ||||||
|
Contributor
There was a problem hiding this comment. And it would be also nice to make component out of this button too
Contributor
There was a problem hiding this comment. Don't forget to use the more explicit notation |
||||||
| anchors.verticalCenter: parent.verticalCenter | ||||||
|
|
||||||
| contentItem: Rectangle { | ||||||
| color: modelData | ||||||
| radius: width / 2 | ||||||
| anchors.fill: parent | ||||||
| } | ||||||
|
|
||||||
| background: Rectangle { | ||||||
| property bool isActive: modelData.toLowerCase() === controller.activeColor.toString().toLowerCase() | ||||||
|
Contributor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment. In map sketching no color is selected anymore by default, while in photo sketching it is |
||||||
|
|
||||||
| anchors.verticalCenter: parent.verticalCenter | ||||||
| anchors.horizontalCenter: parent.horizontalCenter | ||||||
| radius: width / 2 | ||||||
| width: __style.margin48 | ||||||
| height: __style.margin48 | ||||||
| color: isActive ? __style.transparentColor : __style.lightGreenColor | ||||||
| border.width: 2 | ||||||
| border.color: isActive ? __style.grassColor : __style.transparentColor | ||||||
| } | ||||||
|
|
||||||
| onClicked: { | ||||||
| if (showEraseButton) { | ||||||
| controller.eraserActive = false; | ||||||
| } | ||||||
| controller.activeColor = modelData; | ||||||
|
Comment on lines
+48
to
+50
Contributor
There was a problem hiding this comment. again these two should change only stuff in this component and |
||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| MMButton { | ||||||
| text: qsTr("Eraser") | ||||||
| iconSourceLeft: __style.editIcon | ||||||
| // in some instances the erase button is not needed, because there is an "undo" feature already implemented | ||||||
| visible: showEraseButton | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
|
|
||||||
| type: MMButton.Types.Primary | ||||||
| size: MMButton.Sizes.Small | ||||||
|
|
||||||
| fontColor: controller?.eraserActive ? __style.negativeColor : __style.grapeColor | ||||||
| iconColor: controller?.eraserActive ? __style.negativeColor : __style.grapeColor | ||||||
| bgndColor: controller?.eraserActive ? __style.grapeColor : __style.negativeColor | ||||||
| fontColorHover: controller?.eraserActive ? __style.grapeColor : __style.negativeColor | ||||||
| iconColorHover: controller?.eraserActive ? __style.grapeColor : __style.negativeColor | ||||||
| bgndColorHover: controller?.eraserActive ? __style.negativeColor : __style.grapeColor | ||||||
|
|
||||||
| onClicked: { | ||||||
| controller.activeColor = null; | ||||||
| controller.eraserActive = true; | ||||||
| } | ||||||
|
Comment on lines
+64
to
+74
|
||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer calling it
MMColorPicker, it sounds more descriptive and we don't use it for just photos.