[Patch] "No grouping" checkbox in playlist layout

[Nikolaj Hald Nielsen]

Hi Tim

Thanks for the patch. I understand the functionality you want to add
but I need to have a look at your patch and think it over for a little
while as there are other possible way to achieve the effect that you
want that has also been discussed.

In any case, I am off to a big birthday party today, so I will not
have time to look at it before tomorrow at the earliest.

- Nikolaj

On Sat, Aug 29, 2009 at 12:52 AM, Tim Bocek<tim.bocek at gmail.com> wrote:
> Hi,
> Having finished up my thesis I now feel like I have free time to work on
> whatever the hell code I want - and naturally Amarok is the first thing I
> started hacking on :)  So here's a patch - not sure if it's wanted or not,
> but I thought I'd hack it together and email it out just in the nick of time
> for string freeze IIRC.
>
> The patch adds a checkbox to the playlist layout that allows disabling of
> the grouping feature.  Unless I'm missing something, previously users who
> wanted no grouping needed to create an empty header, and duplicate the body
> and single layouts.  Now, they just check this box.
>
> Some questions:
> 1. This is enforced in the UI code currently.  Is this OK, or would it be
> better to enforce somehow in PlaylistLayout.
> 2. Right now, if grouping is disabled and re-enabled, the playlist will have
> the empty header and duplicated body/single layouts.  This might not be
> desireable, e.g. if a user creates a layout with grouping, turns it off, and
> turns it on again.  In this case, would it be more desireable to restore the
> original layouts, or to have the head/body layouts reflect the settings to
> turn off grouping?
> 3. If the previous questions or any other issues require fixes, can we at
> least get the checkbox and associated string in there so this can ship with
> 2.2 (that is, if the patch is even wanted)?
> 4. Since I'm pretty new to FOSS development, anything I should have done
> differently? (in other words, "hey guys im a n00b am i doin this rite?")
>
> Thanks!
> Tim Bocek
>
> PS. Subscribed to the list, no need to CC me
>
> _______________________________________________
> Amarok mailing list
> Amarok at kde.org
> https://mail.kde.org/mailman/listinfo/amarok
>
>