ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Issues:
1) Was there a specific reason to do this? Did you plan on referencing it elsewhere?
2) It'd be tiny, but since we replicate this all over the place it would be nice to make a MusicStyleHints or perhaps put the "1.1" value in the Style.qml file. Actually, maybe create a base MusicState for all these to implement?
3) This bug was fixed in OTA9, you can leave the workaround in if you want, but otherwise we should put together an MP to remove it.
4) Move this commented out code? Or will it be needed when bug is resolved?
5) This is a nitpick, but when we have consecutive updates, let's do 2013-2016, etc. You can leave it as-is for this MP though if you'd like.
6) I know we haven't been consistent, but we should avoid hardcoding this value all the time. Consider moving it to Style.qml
7) Please add parens to clarify the order of operations.
8) Does this deserve it's own component at all? I'd kind of argue that it's not necessary, but having a MusicTabActions component might be "nice" since this logic is rather simple.
9) Move below fill? Occurs a few times in various files.
10) The top of the queue is obscured by the header sections.
11) Only applicable to the new SDK in silo 50, but the header sections seem quite a bit larger.
12) Only applicable to the new SDK in silo 50, but when I select Queue, then select Full view the header disappears.


Resolutions:
1) This is required so that the sidebar can change the colour
2, 6) Styling stuff, I'd like to move to using proper themeing ASAP, so is it best just to leave hardcoded for now?
3) Removed code, please test
4) Fixed
5) I was told somewhere else it is best to specify all the years, and reduces confusion if someone then contributes in 2018 but not 2017 (as that'd have to be 2013-2016, 2018 not 2013-2018). But happy to change if that is what people want.
7) Fixed
8) I'm not sure, as in that component it'd have to reference the id tabs, also I was wondering in an additional MP if I could declare the tabs from a model with repeaters, which would then simplify this code alot.
9) Fixed
10) Fixed
11) "As per design", will probably be the response from upstream
12) Investigating... something weird going on :-)
-- 
https://code.launchpad.net/~ahayzen/music-app/convergence-tabs-with-sidebar-01/+merge/286127
Your team Music App Developers is subscribed to branch lp:music-app.