On Fri, Sep 4, 2009 at 7:28 PM, Oliver
Charles<
oliver.g.charles@...> wrote:
> On Fri, Sep 4, 2009 at 5:35 PM, Aurélien Mino<
aurelien.mino@...> wrote:
>> On Fri, Sep 4, 2009 at 5:31 PM, <
root@...> wrote:
>>>
>>> Author: acid2
>>> Date: 2009-09-04 15:31:00 +0000 (Fri, 04 Sep 2009)
>>> New Revision: 12052
>>>
>>> Added:
>>> mb_server/trunk/root/layout/sidebar/properties.tt
>>> mb_server/trunk/root/layout/sidebar/property.tt
>>> mb_server/trunk/root/layout/sidebar/sidebar-lastupdate.tt
>>> mb_server/trunk/root/layout/sidebar/sidebar-rating.tt
>>> mb_server/trunk/root/layout/sidebar/sidebar-tags.tt
>>> [...]
>>> Removed:
>>> mb_server/trunk/root/components/sidebar-lastupdate.tt
>>> mb_server/trunk/root/components/sidebar-rating.tt
>>> mb_server/trunk/root/components/sidebar-tags.tt
>>> [...]
>>> Log:
>>> Merge Brian's work on the sidebar in the release editor.
>>>
>>> See review 370.
>>>
>>
>> These changes (move sidebar-*.tt from root/components/ to
>> root/layout/sidebar/) were not in
>>
http://codereview.musicbrainz.org/r/370/diff/ as far as I can tell.
>>
>> That breaks a few templates:
>
> Indeed, they were in the release-editor branch that brian was working
> on. I knew about the breaking changes, I could have sworn I commited
> that as well. I'll fix that now, sorry for the break!
I don't think it was a good idea to get this in without a review.
There are still problems:
- recording length in sidebar is formatted incorrectly
("<dt>Length:</dt><dd>225266</dd> ms")
- release labels and catalog numbers disappeared from the release page
- there is a gap between barcode and other release attributes on the
release page
There are also some changes that I don't like functionally. For
example "return unless $self->form;" in FormRendered->_lookup_field is
just masking a problem, we should never have form renderer without a
form. Or that:
<script type="text/javascript">
var MusicBrainz = {};
$(function ($) {
[%#- Hide the edit tab from any browser that
doesn't have javascript enabled (or which is broken for jQuery). -%]
$("#editToggle").show();
});
</script>
should not be in the global template header (or at least returned
conditionally), as it's useful only on release pages.
I also don't like the visual changes in sidebar, but that's subjective
and I can live with them.
--
Lukas Lalinsky
lalinsky@...
_______________________________________________
MusicBrainz-devel mailing list
MusicBrainz-devel@...
http://lists.musicbrainz.org/mailman/listinfo/musicbrainz-devel
