Code review comment for lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/visualGallery

Revision history for this message
Cris Dywan (kalikiana) wrote :

> The first listitem that I click becomes blue, and if I click another listitem
> in the left column after that, it does open the page that I intend to open,
> but still the previously blue listitem stays blue. What is the logic behind
> this?

I don't get it. The code is the same as in the old MR, onClicked pushes the page. Nothing is getting blue here.

> I'll repeat one question from the last MR: is gallery.sh the best name? We
> have another gallery.sh, and it may be confusing (ctrl+k to select files in
> qtc already gives a lot of duplicates for many of the qml files that we have,
> which is very annoying).

It's still the the most obvious name. browser would make sense also but it's just as ambiguous. I don't really have much inspiration here.

> This filter seems random. We don't have a strict policy on the naming,
> and I think only the tests that have a deprecated test version for 1.2
> also have a *13.qml. Plenty of (new) tests do not have 13 in the name,
> like actionbar, adaptivepagelayout, listitem, header, pageheader, sections, toolbar

The filter *seems* random. All new tests being added lately, as far as I'm aware, have a 13 suffix if they use 1.3 and that's what the filter means. Maybe I missed one or two, but I think we'll want to get our naming sorted anyway if we haven't yet. There's no other special requirement. But I'd rather not rename and rewrite more tests in this MR - I patched two to display "something" but some tests need more work to be useful for manual testing. The way I see it, it's not harmful to have those in there, it's motivating to fix them.

« Back to merge proposal