Merge lp://staging/~fabiozaramella/foto/check-album-name into lp://staging/foto/foto-1.0

Proposed by Fabio Zaramella
Status: Merged
Approved by: Erasmo Marín
Approved revision: 108
Merged at revision: 98
Proposed branch: lp://staging/~fabiozaramella/foto/check-album-name
Merge into: lp://staging/foto/foto-1.0
Diff against target: 537 lines (+177/-60)
13 files modified
po/foto.pot (+9/-5)
src/PageContainer.vala (+0/-1)
src/dialogs/AlbumDialog.vala (+4/-2)
src/dialogs/RemoveAlbumDialog.vala (+1/-0)
src/pages/AlbumPage.vala (+3/-3)
src/pages/CollectionPage.vala (+44/-12)
src/pages/LastImportedPage.vala (+2/-2)
src/pages/LibraryPage.vala (+3/-3)
src/pages/Page.vala (+78/-6)
src/pages/TagPage.vala (+3/-3)
src/widgets/ItemSearchBar.vala (+0/-1)
src/widgets/ItemSortBar.vala (+28/-22)
src/widgets/ZoomSlider.vala (+2/-0)
To merge this branch: bzr merge lp://staging/~fabiozaramella/foto/check-album-name
Reviewer Review Type Date Requested Status
Erasmo Marín Approve
Review via email: mp+234891@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-09-14.

Description of the change

p.s. Rewriting the Sort toolbar will take me a while since i don't have so much time this week (school :/) however i reported some other bugs

To post a comment you must log in.
Revision history for this message
Erasmo Marín (erasmo-marin) wrote : Posted in a previous version of this proposal

in diff, line 107, there is a commented line. Fix that and we are ready to go. Works pretty well.

Revision history for this message
Erasmo Marín (erasmo-marin) wrote : Posted in a previous version of this proposal

are you going to make more changes or this is the final version to merge?

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote : Posted in a previous version of this proposal

The only thing that miss is to fix the problem that cause sort and search buttons to be sensitive when switching from an album with photos to one without but i think it will fix automatically if on empty album se will offer the import option, however you can merge if you want, they can be solved later.

Revision history for this message
Erasmo Marín (erasmo-marin) wrote : Posted in a previous version of this proposal

There are some details I dislike. For example, the searchbar has no style and the hide animation in the sort bar is different. Also, I think all these buttons at the right looks bad, maybe moving some elements to the left toolbar? The import and sort button maybe.

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote : Posted in a previous version of this proposal

What do you mean with animations? It's the dame for both the toolbar, if you mean the close animation i didn't understand if is possibile to do it, i'll investigate.
The searchbar has a style, the same style of the sort toolbar, if you mean the search entry, yes i'll update it as soon as i can.

At least, my idea was to move the search and the sort buttons to the right and leave the import button on the left, this would make much more sense, but in order to do that we need to remove the selection menu from headerbar and put it into a container on the bottom, as i reported.

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote : Posted in a previous version of this proposal

Same* ops the corrector

Revision history for this message
Erasmo Marín (erasmo-marin) wrote : Posted in a previous version of this proposal

The problem with animations are that I would like to have only 1 bar at the same time, not 2, so, if I click search, if the sort bar is enabled, it should be hidden. And also, the problem with gtk revealer and hide animation is an issue too.
About the buttons position, you are right, we should fix the selection menu first. Right now I'm very busy with college stuff, so can't promise to do it soon, but I will do it as soon as possible. I will remove the text buttons, so the buttons "flashing" problem will be solved. Instead I will put a "multiple selection" button (like this http://debarshiray.wordpress.com/2013/08/09/gnome-photos-3-9-x ).

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote : Posted in a previous version of this proposal

A button for multiple selection would be a good thing, however i made i istake. I meant to have a toolbar like this:

search | sort | multiple selection ... title ... slider | import

Revision history for this message
Erasmo Marín (erasmo-marin) wrote : Posted in a previous version of this proposal

Yep, I was thinking in something very similar. Search and sort are very similar functions, so they should be together. I like more the search at right, but it will look cluttered. Other way would be to put the search entry in the sort bar, after all, there is enough space.

What about doing something like this? just an idea

import | contextual buttons (ie: remove) | multiple selection ... title ... slider | sort and search

Revision history for this message
Fabio Zaramella (fabiozaramella) wrote : Posted in a previous version of this proposal

I think that at the moment we can keep two separates toolbars, we can always change this later. However i think that the titlerbar should be:

"multiple selection" | search | sort ... title ... slider | import

Because the import button is usually on the right.

But to make the titlebar look like above we have to remove the selection menu and pack it into the container on the bottom (that would appear when one is selected). So i won't made other changes to this branch for the moment, merge it if you think it is ok now.

Revision history for this message
Erasmo Marín (erasmo-marin) wrote :

great!! I will merge my branch first and then this one

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: