Files

Merge lp://staging/~jeremywootten/pantheon-files/refactor-location-bar-part2 into lp://staging/~elementary-apps/pantheon-files/trunk

  1. refactor-location-bar-part2
  2. Merge into trunk
Proposed by Jeremy Wootten
Status: Merged
Approved by: Danielle Foré
Approved revision: 1980
Merged at revision: 1982
Proposed branch: lp://staging/~jeremywootten/pantheon-files/refactor-location-bar-part2
Merge into: lp://staging/~elementary-apps/pantheon-files/trunk
Prerequisite: lp://staging/~jeremywootten/pantheon-files/refactor-location-bar-preparation
Diff against target: 5596 lines (+2592/-2106)
32 files modified
filechooser-module/CMakeLists.txt (+13/-8)
filechooser-module/FileChooserDialog.vala (+9/-9)
libcore/CMakeLists.txt (+0/-3)
libcore/pantheon-files-core-C.vapi (+5/-0)
libwidgets/CMakeLists.txt (+22/-26)
libwidgets/Chrome/BasicBreadcrumbsEntry.vala (+657/-0)
libwidgets/Chrome/BasicLocationBar.vala (+137/-0)
libwidgets/Chrome/BreadcrumbElement.vala (+253/-0)
libwidgets/Chrome/BreadcrumbIconList.vala (+152/-0)
libwidgets/Chrome/BreadcrumbsElements.vala (+0/-170)
libwidgets/Chrome/LocationBar.vala (+0/-881)
libwidgets/Chrome/TopMenu.vala (+35/-15)
libwidgets/Chrome/ViewWindowInterface.vala (+0/-35)
libwidgets/Chrome/Viewable.vala (+0/-32)
libwidgets/Dialogs/ChooseAppDialog.vala (+6/-2)
libwidgets/FileUtils.vala (+28/-1)
libwidgets/Interfaces/LocatableInterface.vala (+26/-0)
libwidgets/Interfaces/NavigatableInterface.vala (+43/-0)
libwidgets/Interfaces/SearchableInterface.vala (+33/-0)
libwidgets/MimeActions.vala (+22/-0)
libwidgets/Resources.vala (+11/-2)
libwidgets/View/BreadcrumbsEntry.vala (+548/-0)
libwidgets/View/LocationBar.vala (+189/-558)
libwidgets/View/SearchResults.vala (+250/-245)
plugins/contractor/CMakeLists.txt (+5/-0)
src/CMakeLists.txt (+1/-0)
src/View/AbstractDirectoryView.vala (+1/-1)
src/View/AbstractTreeView.vala (+10/-8)
src/View/DirectoryNotFound.vala (+3/-2)
src/View/IconView.vala (+8/-7)
src/View/ViewContainer.vala (+50/-29)
src/View/Window.vala (+75/-72)
To merge this branch: bzr merge lp://staging/~jeremywootten/pantheon-files/refactor-location-bar-part2
Reviewer Review Type Date Requested Status
Danielle Foré Approve
Adam Bieńkowski (community) functionality Approve
Corentin Noël Approve
xapantu Pending
Review via email: mp+276330@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-10-21.

Commit message

Refactor pathbar widgets to give browser-like behaviour

Description of the change

The pathbar widgets in /libwidgets have been refactored to make them more modular and to achieve a more browser-like behaviour. Explicit interfaces between the widgets have been defined.

The need for a separate filechooser widget has been removed; the base BreadcrumbEntry widget can be used instead.

A number of existing bugs in the old widgets have been addressed along the way.

This is a remade version of the branch using bzr mv to put old widget files into new positions (and names) before updating them to the equivalent new widget. The intermediate revisions are not buildable. Because of the substantial and extensive changes made, the total diff is still long.

To post a comment you must log in.
Revision history for this message
xapantu (xapantu) wrote : Posted in a previous version of this proposal

Please use bzr mv when you move files, the diff is not readable.

Please split your changes into different branch, I can't see why the build system is touched here.

But of course, as always, it looks like good work ;)

review: Disapprove
Revision history for this message
Jeremy Wootten (jeremywootten) wrote : Posted in a previous version of this proposal

xapantu: Thanks for the review. I think most of the changes to the CMakeList.txt files were required to make the branch compile although e.g. r1974 and r1975 may not be strictly essential for this branch. Which changes are you disapproving?

I have split the intended changes into three parts already (see parts 1 and 3 also proposed), but this part is not so easy to split, while maintaining buildability and full functionality, as most of the changes are interdependent. Do you have any advice as to how this branch may be further split?

I'll remake the branch using bzr mv as suggested.

Revision history for this message
Corentin Noël (tintou) wrote :

This is indeed impossible to really review this, but this changes are needed for Files so I'm more into merging it and fixing the remaining bug via some dogfooding.

review: Approve
Revision history for this message
Cody Garver (codygarver) wrote :

Be sure to include the changes found in trunk commit r1978

http://bazaar.launchpad.net/~elementary-apps/pantheon-files/trunk/revision/1978

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

As tintou said, I would like to merge it and I can't review the merge fully since the diff is huge... I tested some cases with it, and it seemed to work fine for me.

review: Approve (functionality)
Revision history for this message
Danielle Foré (danrabbit) wrote :

Looks like it conflicts with trunk currently

review: Needs Fixing
lp://staging/~jeremywootten/pantheon-files/refactor-location-bar-part2 updated
1978. By Jeremy Wootten

Merge parent r1958 (trunk r1979)

1979. By Jeremy Wootten

Fix bug in completion on remote filesystem

1980. By Jeremy Wootten

Merge parent (trunk r1981)

Revision history for this message
Danielle Foré (danrabbit) wrote :

Confirmed that the linked issues were fixed

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: