Deere skin: Add most missing tooltips in attempt to fix https://bugs.… #1500

esbrandt · 2018-01-27T14:33:24Z

…launchpad.net/mixxx/+bug/1745670

Left out the tooltip for the samplers CUE button. I must have missed what the purpose of the button is. It is exclusively to the Deere skin, was introduced for 2.1, and behaves different then the decks CUEbutton.

…launchpad.net/mixxx/+bug/1745670

daschuer · 2018-01-27T22:51:53Z

Thank you. I have bin also stumbled over this CUE button.
@be: does it work like intended?

ronso0 · 2018-01-30T01:56:57Z

I also don't really understand why we have that Cue button in Deere only.
Other skins have the following behaviour mapped to samplers' Play button which is distinct and sufficient IMO (but I didn't find a matching tooltip for that either):
left-click: cue_gotoandplay
right-click: cue_default
display: play_indicator

This allows to always and instantly play from prepared cue point, right-click resets to cue point.
To set a new cue point, right-click Play, go to a hotcue, right-click Play to set the new cue point.
Play from there.

lp:1743109 is related.

…ed size waveforms, and only displayed 4 hotcues by default. Fixes https://bugs.launchpad.net/mixxx/+bug/1750210

…ady used in Tango skin) , for accessing the skin settings menu. Remove static english button label, opt-in for translatable tooltip instead.

…mplers . This fixes a regression from 2.0 . Tooltips for the buttons still missing, but we are in string freeze for 2.1.

daschuer · 2018-02-19T16:49:48Z

Tooltips for the buttons still missing, but we are in string freeze for 2.1.

We may argue that English tooltips are better than none.

daschuer · 2018-02-19T16:54:03Z

LGTM!
Is this ready for merge

esbrandt · 2018-02-19T18:47:11Z

We may argue that English tooltips are better than none.

NP, i´ll add them.

LGTM!
Is this ready for merge

I`d like to throw out the samplers CUE buttons first, since nobody seems to know the intended use case.

…lusive to Deere. It was introduced post-2.0, but nobody could say clearly why, or what the intended use was.

esbrandt · 2018-02-20T08:23:26Z

src/skin/tooltips.cpp

@@ -869,4 +869,13 @@ void Tooltips::addStandardTooltips() {
    add("skin_settings")
            << tr("Skin Settings Menu")
            << tr("Show/hide skin settings menu");
+
+    // Sampler Bank Controls


The tooltips are short. If you feel they should be more in detail, e.g. that technical not the samples are saved, please speak up.

esbrandt · 2018-02-20T08:24:26Z

res/skins/Deere/sampler_controls_row.xml

@@ -77,19 +77,6 @@
        <Layout>horizontal</Layout>
        <ObjectName>ButtonGrid</ObjectName>
        <Children>
-
-          <Template src="skin:left_display_2state_button.xml">


I plan to work a bit more on the samplers layout, but this must not be done in this PR

@esbrandt Will you take care of this before 2.1 release or should I?
https://bugs.launchpad.net/mixxx/+bug/1743109 "unify Sampler interface in all skins" is almost fixed, except Deere samplers are lacking hotcues. Hotcues 5-8 would need to be in a second hotcue row on small screens (this can be constructed responsive, like some things in Tango)

…these hotcues´ text would align to the bottom of the main waveform, while the rest would align to the top.

…idget, since legacyskinparser throws error ``Could not parse widget MaximumSize: "-1,-1" ``. No change in functionality.

… library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816

esbrandt · 2018-02-21T11:14:49Z

This has become a pool of skin related fixes. I would like to continue in separate PRs, and have this reviewed and merged.

ronso0 · 2018-02-22T00:19:35Z

src/library/playlistfeature.cpp

@@ -248,6 +248,10 @@ QString PlaylistFeature::getRootViewHtml() const {
                                            playlistsSummary4));
    html.append("</td></tr>");
    html.append(QString("<tr><td><a href=\"create\">%1</a>")


this line is obsolete, isn't it? <tr><td> go to next line then
build fails due to missing ) and ;

ronso0 · 2018-02-22T00:41:44Z

src/library/playlistfeature.cpp

+    //Colorize links in lighter blue, instead of QT default dark blue.
+    //Links are still different from regular text, but readable on dark/light backgrounds.
+    //https://bugs.launchpad.net/mixxx/+bug/1744816
+    html.append(QString("<a style=\"color:#0850D0;\" href=\"create\">%1</a>")


it works and looks like desired if the line is

html.append(QString("<tr><td><a style=\"color:#0850D0;\" href=\"create\">%1</a>") .arg(createPlaylistLink));

fixed with 83b515e

fixed with 83b515e

What is going on here, to which branch went this, and the other commits?
I can´t find them..

Oh, sorry for being unclear. I made those commits to my fork of your 2.1 branch deere-crate-link-fix-fixed which you could just merge into yours. It contains:

create crate/playlist: fix html link

make skin settings icon a bit smaller

settings icon: remove SVG attributes 'width' and 'height' so it resized properly in the skin

use skin settings tooltip in Tango

ronso0 · 2018-02-22T00:59:20Z

The settings icon could be a bit smaller, then it would match the Recording and Broadcast icons:

check 665a950

Edit
and 0a91897 which fixes the resizing issue with SVG attributes 'width' and 'height'

ronso0 · 2018-02-22T01:12:36Z

In scrolling waveforms, there's no CUE mark anymore.
The cue/hotcue marker labels are centered. If a deck is stopped at a cue point its label is hidden behind the playposition marker. We can put it to the top right corner:

        <DefaultMark>
          <Align>top|right</Align>
          <Color>#00FF00</Color>
          <TextColor>#000000</TextColor>
          <Text> %1 </Text>
        </DefaultMark>

ronso0 · 2018-02-22T01:27:00Z

Skin settings tooltip for Tango: a9c1b6b

Be-ing · 2018-02-22T13:46:47Z

res/skins/Deere/deck_visual_row.xml

          <Text>1</Text>
          <Align>bottom</Align>
          <Color>#00FF00</Color>
          <TextColor>#FFFFFF</TextColor>
        </Mark>
        <Mark>
          <Control>hotcue_2_position</Control>
-          <Pixmap>image/marker_hotcue_2.png</Pixmap>
+          <Pixmap></Pixmap>


Can this empty element be removed?

Be-ing · 2018-02-22T13:48:35Z

Thank you for the fixes and sorry for the delayed review. Please fix the issue with the cue marker that @ronso0 mentioned, otherwise LGTM. It may be nice to add a little more padding on the left and right sides of the Mixxx logo, but be careful that everything would still fit in the minimum width.

…s of the library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816" This reverts commit 9f10e47.

… library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816 . 2nd try, this time without introducing build errors.

… pages of the library view. Remove clipart in CRATE page. We got used to it, but it had no real benefit.

…page of the library view as well.

…ettings menu

…ly deleted in cddabf1

esbrandt · 2018-02-23T16:19:24Z

The settings icon could be a bit smaller, then it would match the Recording and Broadcast icons:

check 665a950

Edit
and 0a91897 which fixes the resizing issue with SVG attributes 'width' and 'height'

I´d like to save icon work for another branch. Has the resizing issue been reported before?

esbrandt · 2018-02-23T16:22:39Z

res/skins/Deere/deck_visual_row.xml

@@ -66,6 +66,15 @@
        - if a MIDI device which supports more hotcues than buttons are in the current skin has them activated
        - if you change from a skin which supports more hotcues than buttons are in the current skin (and has them activated)
        -->
+        <Mark>


I restored to the initial, working state. Personally, i think the graphics should be removed as well for loops and CUE points, but there was no feedback yet in the original bug report https://bugs.launchpad.net/mixxx/+bug/1750210

What would you replace them with? A letter?

I do think the default rendering suits best - partly because of limitations in the subpixel rendering of the images.

For hotcues we allow to name hotcues (for now only in CUE tab of the tracks´ preferences). Adding an image will overwrite the hotcues´ label.
So + 1 for me here for default rendering.

For Loop-In, Loop-Out, and CUE points, i do think using images is not preferable for now, because of errors in the rendering of the images.

You can see it clearly when you have quantize enabled, and trying to exactly set a CUE image on a beat-grid. Even if your image is only 1px wide, it gets randomly placed left, right, or directly on the beat-grid line.

Skins that currently use marker images dodge this by making the vertical line off center, or 2+ pixel wide.

ronso0 · 2018-02-23T21:01:41Z

Has the resizing issue been reported before?

No, I thought it's a Inkscape issue. Couldn't reproduce so far, because I don't recall with which svg options I saved the file initially, after editing.
Basically the issue pops up if the svg contains

width="100%"
height="100%"

ronso0 · 2018-02-23T21:22:58Z

Personally, i think the graphics should be removed as well for loops and CUE points,

I agree. Loop range icons could either be removed or simplified so that they are just filled squares, somehow resembling the respective button icons.

ronso0 · 2018-02-24T19:23:14Z

Crates & Playlists main pages look good! I don't miss the clip art since I don't use those pages anyway.

In Playlists main page, the lines in third paragraph are pretty long (english, translations might be even longer). Can we introduce a line-break after ~600px so they're easier to read? Or would that change string translations?

…IST page of the Library view. Improves readability a bit on wide screens. No effect for i18n.

esbrandt · 2018-02-25T07:35:14Z

Can we introduce a line-break after ~600px so they're easier to read? Or would that change string translations?

Added a simple line break between the two sentences of the paragraph. No effect for translations.

No, I thought it's a Inkscape issue. Couldn't reproduce so far, because I don't recall with which svg options I saved the file initially, after editing.
Basically the issue pops up if the svg contains
...

Can you please report the bug, and how to reproduce in Mixxx if possible?

Be-ing · 2018-02-25T12:37:16Z

src/library/crate/cratefeature.cpp

+    //Colorize links in lighter blue, instead of QT default dark blue.
+    //Links are still different from regular text, but readable on dark/light backgrounds.
+    //https://bugs.launchpad.net/mixxx/+bug/1744816
+    html.append(QString("<tr><td><a style=\"color:#0850D0;\" href=\"create\">%1</a>")


I think this could be brighter still.

#0850D0
looks pretty readable to me ( on OSX system-wide available as AlternateSelectedControlColor, used in Mixxx when selecting a library item.)
#0496FF would be a similar color, if you think this still has to change.

#0496FF is easy to read even with my screen brightness all the way down 👍

…LAYLIST pages of the library view even brighter. Follow up to 303fe94

Deere skin: Add most missing tooltips in attempt to fix https://bugs.…

9d9a5bf

…launchpad.net/mixxx/+bug/1745670

esbrandt added 4 commits February 18, 2018 08:57

Merge remote-tracking branch 'upstream/2.1' into 2.1

f384f81

Remove static images for Hotcues 1-4. They made sense when we had fix…

99510f4

…ed size waveforms, and only displayed 4 hotcues by default. Fixes https://bugs.launchpad.net/mixxx/+bug/1750210

Deere: Replace controversial Hamburger icon with a gear icon (as alre…

30aa6bd

…ady used in Tango skin) , for accessing the skin settings menu. Remove static english button label, opt-in for translatable tooltip instead.

Deere: Re-add option to load/save sampler banks to Skin settings > Sa…

38c6462

…mplers . This fixes a regression from 2.0 . Tooltips for the buttons still missing, but we are in string freeze for 2.1.

esbrandt added 2 commits February 20, 2018 08:50

Add tooltips for Sampler Bank controls. Use them in Tango and Deere

e9da243

Deere: Remove CUE button from samplers controls row. This was exc…

d119889

…lusive to Deere. It was introduced post-2.0, but nobody could say clearly why, or what the intended use was.

esbrandt commented Feb 20, 2018

View reviewed changes

esbrandt added this to the 2.1.0 milestone Feb 20, 2018

esbrandt added the skins label Feb 20, 2018

esbrandt added 5 commits February 20, 2018 14:50

Deere: Do not tread Hotcues 1-4 different than the rest. Previously, …

cddabf1

…these hotcues´ text would align to the bottom of the main waveform, while the rest would align to the top.

Deere: Remove infinite <MaximumSize> key in decks waveform overview w…

b3c49b4

…idget, since legacyskinparser throws error ``Could not parse widget MaximumSize: "-1,-1" ``. No change in functionality.

Merge remote-tracking branch 'upstream/2.1' into 2.1

3ce7e68

Merge remote-tracking branch 'upstream/2.1' into 2.1

cc7d8ae

Skins: Fix hard to read link color in CRATE and PLAYLIST pages of the…

9f10e47

… library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816

ronso0 reviewed Feb 22, 2018

View reviewed changes

Be-ing reviewed Feb 22, 2018

View reviewed changes

Revert "Skins: Fix hard to read link color in CRATE and PLAYLIST page…

4e53e11

…s of the library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816" This reverts commit 9f10e47.

esbrandt added 5 commits February 23, 2018 09:27

Skins: Fix hard to read link color in CRATE and PLAYLIST pages of the…

303fe94

… library view. Fixes https://bugs.launchpad.net/mixxx/+bug/1744816 . 2nd try, this time without introducing build errors.

Removed waste text formatting using table cells in CRATE and PLAYLIST…

7f3b1d3

… pages of the library view. Remove clipart in CRATE page. We got used to it, but it had no real benefit.

Removed waste text formatting using table cells in COMPUTER (BROWSE) …

cde2bdf

…page of the library view as well.

Tango: Add TooltipID for skin settings buttons in top bar, and skin s…

8073569

…ettings menu

Deere: Fix CUE point being invisible on main waveform, was accidental…

9381ad9

…ly deleted in cddabf1

esbrandt commented Feb 23, 2018

View reviewed changes

esbrandt added 2 commits February 25, 2018 08:19

Add line break in 3rd paragraph of the long explanatory text in PLAYL…

3197e06

…IST page of the Library view. Improves readability a bit on wide screens. No effect for i18n.

Merge remote-tracking branch 'upstream/2.1' into 2.1

8f52fdf

Be-ing reviewed Feb 25, 2018

View reviewed changes

Skins: Based on feedback, make hard to read link color in CRATE and P…

2d06a30

…LAYLIST pages of the library view even brighter. Follow up to 303fe94

Be-ing merged commit 1ef9347 into mixxxdj:2.1 Feb 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deere skin: Add most missing tooltips in attempt to fix https://bugs.… #1500

Deere skin: Add most missing tooltips in attempt to fix https://bugs.… #1500

esbrandt commented Jan 27, 2018 •

edited

daschuer commented Jan 27, 2018

ronso0 commented Jan 30, 2018 •

edited

daschuer commented Feb 19, 2018

daschuer commented Feb 19, 2018

esbrandt commented Feb 19, 2018

esbrandt Feb 20, 2018

esbrandt Feb 20, 2018

ronso0 Mar 5, 2018 •

edited

esbrandt commented Feb 21, 2018

ronso0 Feb 22, 2018 •

edited

ronso0 Feb 22, 2018

ronso0 Feb 22, 2018

esbrandt Feb 22, 2018 •

edited

ronso0 Feb 22, 2018

ronso0 commented Feb 22, 2018 •

edited

ronso0 commented Feb 22, 2018

ronso0 commented Feb 22, 2018

Be-ing Feb 22, 2018

Be-ing commented Feb 22, 2018

esbrandt commented Feb 23, 2018 •

edited

esbrandt Feb 23, 2018

Be-ing Feb 25, 2018

esbrandt Feb 25, 2018

ronso0 commented Feb 23, 2018

ronso0 commented Feb 23, 2018

ronso0 commented Feb 24, 2018

esbrandt commented Feb 25, 2018

Be-ing Feb 25, 2018

esbrandt Feb 25, 2018

Be-ing Feb 25, 2018 •

edited

Deere skin: Add most missing tooltips in attempt to fix https://bugs.… #1500

Deere skin: Add most missing tooltips in attempt to fix https://bugs.… #1500

Conversation

esbrandt commented Jan 27, 2018 • edited

daschuer commented Jan 27, 2018

ronso0 commented Jan 30, 2018 • edited

daschuer commented Feb 19, 2018

daschuer commented Feb 19, 2018

esbrandt commented Feb 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronso0 Mar 5, 2018 • edited

Choose a reason for hiding this comment

esbrandt commented Feb 21, 2018

ronso0 Feb 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esbrandt Feb 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronso0 commented Feb 22, 2018 • edited

ronso0 commented Feb 22, 2018

ronso0 commented Feb 22, 2018

Choose a reason for hiding this comment

Be-ing commented Feb 22, 2018

esbrandt commented Feb 23, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronso0 commented Feb 23, 2018

ronso0 commented Feb 23, 2018

ronso0 commented Feb 24, 2018

esbrandt commented Feb 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing Feb 25, 2018 • edited

Choose a reason for hiding this comment

esbrandt commented Jan 27, 2018 •

edited

ronso0 commented Jan 30, 2018 •

edited

ronso0 Mar 5, 2018 •

edited

ronso0 Feb 22, 2018 •

edited

esbrandt Feb 22, 2018 •

edited

ronso0 commented Feb 22, 2018 •

edited

esbrandt commented Feb 23, 2018 •

edited

Be-ing Feb 25, 2018 •

edited