Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User can see close focused project folder, alphabetize folders, and collapse folders. #657

Conversation

roymckenzie
Copy link
Contributor

@cassidyjames I've updated this branch with some of the ideas we messaged about. Specifically:

  1. The ability to close an active / selected Project Folder.
  2. The ability to collapse all Project Folders.
  3. The ability to order Project Folders alphabetically.

Let me know what you think.

Screenshot from 2019-03-13 03 36 05

@tintou
Copy link
Member

tintou commented Mar 13, 2019

Wait, shouldn't the project folder always be alphabetized?

@roymckenzie
Copy link
Contributor Author

@tintou they didn't appear to be alphabetized by default. If that is something that should be done by default (probably), I can modify the add_folder method in FileView to do that and we can just remove it from the Gtk.Menu in the inline toolbar.

@cassidyjames
Copy link
Contributor

@tintou @roymckenzie to clarify, this is about sorting the projects themselves in alphabetical order, not the contents of the projects/folders. In this case, no, they should not always be alphabetized because they're in the order they were added; you know the last-added project will always be at the bottom. However, if you've amassed a large amount of projects you're jumping between a lot, it would be nice to sort them in alphabetical order.

@roymckenzie
Copy link
Contributor Author

Looking at the icon for the remove project folder action, I don't like the icon that much. It is hard to see and does not seem intuitive.

I am currently using the folder-move-symbolic icon for that.

A deeper question I have is: Does it even make since to have that button / functionality located there? The user can right-click on the project folder in the FileView to close a specific project folder. Actions on the newly proposed inline toolbar imply more general actions related to all project folders than one specific one in my opinion.

@cassidyjames
Copy link
Contributor

@roymckenzie that's fair. It's also not clear which specific project is "selected", so removing one could be problematic imho.

@roymckenzie
Copy link
Contributor Author

@cassidyjames I removed the "Close Active Folder" functionality from the inline toolbar (and this branch). Let me know what you think.

Copy link
Contributor

@cassidyjames cassidyjames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes. I also wonder… would it make sense to remove the now-redundant "open a folder" item from the headerbar?

src/Widgets/Pane.vala Outdated Show resolved Hide resolved
var add_button = new Gtk.ToolButton (new Gtk.Image.from_icon_name ("folder-open-symbolic", Gtk.IconSize.BUTTON), null);
add_button.action_name = Scratch.MainWindow.ACTION_PREFIX + Scratch.MainWindow.ACTION_OPEN_FOLDER;
add_button.tooltip_text = _("Add Project Folder…");
var add_folder_button = new Gtk.ToolButton (new Gtk.Image.from_icon_name ("folder-open-symbolic", Gtk.IconSize.BUTTON), null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we're setting this to a symbolic icon, but for some reason it's not rendering as symbolic… any idea what's up here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danrabbit mentioned this is due to the stylesheet, and not something within Code.

src/Widgets/Pane.vala Outdated Show resolved Hide resolved
src/Widgets/Pane.vala Outdated Show resolved Hide resolved
src/Widgets/Pane.vala Outdated Show resolved Hide resolved
cassidyjames and others added 4 commits March 29, 2019 00:17
Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>
Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>
Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>
Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>
@jeremypw
Copy link
Collaborator

@roymckenzie Will you be updating this PR? There seems to be an outstanding change request. If so, you might consider incorporating the functionality of #665 (if feasible).

@cassidyjames cassidyjames requested a review from a team October 3, 2019 17:51
@cassidyjames
Copy link
Contributor

Barring that symbolic issue, I'm personally very happy with this branch and have been using it for a while. I forgot this wasn't in master! 😅

@cassidyjames
Copy link
Contributor

Ah, this should probably also remove the Open Folder button from the HeaderBar since it could be confusing that there are two places to do it. But I'd want to get thoughts from @elementary/ux.

@cassidyjames
Copy link
Contributor

@roymckenzie it turns out the icon issue is actually an issue in the stylesheet, so I've reported it there.

@cassidyjames cassidyjames dismissed their stale review October 3, 2019 18:02

Not an issue in this repo

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See in line comments.
@cassidyjames : Did you still want the headerbar "add folder" button removed here? You may want to override my comments on the icons.

toolbar.add (add_button);
var project_more_button = new Gtk.MenuToolButton (null, null);
project_more_button.tooltip_text = _("Manage project folders…");

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint: Unexpected whitespace at end of line.

var add_button = new Gtk.ToolButton (new Gtk.Image.from_icon_name ("folder-open-symbolic", Gtk.IconSize.BUTTON), null);
add_button.action_name = Scratch.MainWindow.ACTION_PREFIX + Scratch.MainWindow.ACTION_OPEN_FOLDER;
add_button.tooltip_text = _("Add Project Folder…");
var add_folder_button = new Gtk.ToolButton (new Gtk.Image.from_icon_name ("folder-open-symbolic", Gtk.IconSize.BUTTON), null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint: Long line

src/Widgets/Pane.vala Show resolved Hide resolved

toolbar.add (add_button);
var project_more_button = new Gtk.MenuToolButton (null, null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect a "open-menu" symbolic icon rather than the default down arrow which I associate with downloading or a combobox. A "hamburger" type icon would be nice but I am not sure that one is available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just using the default icon which I believe is used in other similar "dropdown" contexts, but I'd be fine to change to something else. I wanted to avoid using a settings-type icon because this is not necessarily settings, it's actions. view-more-symbolic might work. Thoughts from @elementary/ux?

@cassidyjames
Copy link
Contributor

Did you still want the headerbar "add folder" button removed here?

@jeremypw I think so, because having it in both places doesn't make a ton of sense to me (and just muddies sidebar actions with global actions). I did remove it from the HeaderBar in a commit above, but I'd be open to @elementary/ux feedback as well.

@jeremypw
Copy link
Collaborator

@cassidyjames I have removed the headerbar "New folder" button and used the same icon in the inline toolbar as users will be familiar with its meaning in this context.

@cassidyjames
Copy link
Contributor

@jeremypw we should not use the non-semantic icon, the open-folder icon is more correct.

@jeremypw
Copy link
Collaborator

@cassidyjames OK. I now notice that this PR is merging into another older PR of yours anyway - I am confused as to which to work on. I probably should not have pulled in the master branch - there is now an enormous diff for some reason :-(

@cassidyjames
Copy link
Contributor

@jeremypw ah I didn't realize that. I say we merge this into my original branch since I'm on board with the changes so far and then we can use that as the actual PR. Sorry about the mess!

@cassidyjames cassidyjames merged commit 810c9b0 into elementary:sidebar-inline-toolbar Nov 19, 2019
jeremypw pushed a commit that referenced this pull request Nov 25, 2019
* Add inline toolbar to sidebar

* Remove Open Folder button from HeaderBar

Since it's redundant

* User can see close focused project folder, alphabetize folders, and collapse folders. (#657)

* User can see close focused project folder, alphabetize folders, and collapse folders.

* removed close folder icon/functionality in the inline toolbar

* Update src/Widgets/Pane.vala

Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>

* Update src/Widgets/Pane.vala

Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>

* Update src/Widgets/Pane.vala

Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>

* Update src/Widgets/Pane.vala

Co-Authored-By: roymckenzie <1065321+roymckenzie@users.noreply.github.com>

* Move headerbar open folder button to inline toolbar

* Fix lint errors

* Revert icon change

* Fix lint error (again)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants