[Sugar-devel] [PATCH] Fixes to the record UI

[Anish Mangal]

Hi Daniel,

Thanks for the feedback. I'm working on making this patch better as I
gain a better understanding of how/what is going wrong. Meanwhile,
find my responses to your queries inlined.

> OK.
> So this patch introduces some "flashing" ?

It doesn't introduce any 'new' flashing since those widgets were
already visible with the broken UI. If you were to compare this with
the existing record on 0.84, then the answer would be yes.

> The resizing 1x1 thing is ugly, I'd rather we understood this issue
> and came up with something nicer.

After our discussion on #sugar yesterday, I think its pretty clear
that 1x1 isn't the right way forward. This still under work.

> Lets retain the previous behaviour. I think if you took a photo in
> full screen mode it would stay like that. Right?

Yes, it stays like that.

> GTK+ is pretty good at widget layout. The problem is probably the way
> we render the preview window being outside of GTK's layout
> considerations. I'd suggest leaving it  as-is and looking for a better
> way to fix this for a future patch.

Agree, this should be a separate patch.

Cheers,
Anish



On Wed, Jun 9, 2010 at 8:20 PM, Daniel Drake <dsd at laptop.org> wrote:
> On 8 June 2010 23:25, Anish Mangal <anishmangal2002 at gmail.com> wrote:
>> Hi,
>>
>> I did reply to your mail earlier today, I'll paste my comments here ...
>>
>>> Still unanswered questions from my last mail:
>>> There was probably a reason why it moved them off screen instead of hiding them.
>>> Do you have any idea what it is?
>>
>> This comment in ui.py explains why
>>       #we move offscreen to resize or else we get flashes on screen,
>> and setting hide() doesn't allow resize & moves
>
> OK.
> So this patch introduces some "flashing" ?
>
>>> Also, if you are hiding widgets, I don't see any reason to resize them to 1x1.
>>
>> For some widgets, when I attempt to hide them, the UI doesn't work, so
>> I have to resize them to 1x1.
>> (I only resize only those widgets which I can't hide, so there is no redundancy)
>
> The resizing 1x1 thing is ugly, I'd rather we understood this issue
> and came up with something nicer.
>
>>> Comments for other reviewers: for me, the patch is now harder to
>>> review because it contains a load of unrelated changes renaming
>>> variables. I agree with the name changes, but perhaps best to be
>>> careful with feedback this way.
>>
>> Sorry, I'll be careful to include separate changes in different
>> patches the next time.
>
> This wasn't your fault.
>
>> No problem here. This seemed like a minor quirk to me when compared
>> with recording videos in fullscreen. When we finish recording a video
>> in fullscreen mode, we are automatically taken out of fullscreen mode.
>> However in the case of taking photos, when we click the shutter, the
>> photo is captured but we remain in full screen mode. This small
>> snippet takes us out of fullscreen after capturing the photo. We can
>> totally do without this code as well and the UI would work just fine.
>
> Lets retain the previous behaviour. I think if you took a photo in
> full screen mode it would stay like that. Right?
>
>> When running record on smaller screen resolutions (or perhaps even xo
>> resolution), in the info view, the preview window slightly overlaps
>> with the tags and date text fields. This snippet attempts to fix that.
>
> GTK+ is pretty good at widget layout. The problem is probably the way
> we render the preview window being outside of GTK's layout
> considerations. I'd suggest leaving it  as-is and looking for a better
> way to fix this for a future patch.
>
> Daniel
>