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

search-in-workspace: update 'collapse' toolbar item to also 'expand' #7714

Closed
vince-fugnitto opened this issue May 1, 2020 · 11 comments · Fixed by #9749
Closed

search-in-workspace: update 'collapse' toolbar item to also 'expand' #7714

vince-fugnitto opened this issue May 1, 2020 · 11 comments · Fixed by #9749
Labels
enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace

Comments

@vince-fugnitto
Copy link
Member

vince-fugnitto commented May 1, 2020

Description

The 'collapse all' toolbar item for the search-in-workspace widget should be updated to also support expanding nodes (when the tree has been collapsed) similarly to VS Code. The toolbar item should take care of toggling the expansion of the tree between collapsed/expanded.

In the following screenshots notice the last toolbar item (+/- based on the tree state):

Collapsed:

Screen Shot 2020-05-01 at 8 24 58 AM

Expanded:

Screen Shot 2020-05-01 at 8 25 11 AM

Steps to Reproduce

  1. perform a search using the search-in-workspace widget
  2. execute the 'collapse all' toolbar item (tree is collapsed and there is no way to expand all)
  3. perform the same use-case in VS Code (collapse all becomes expand all when the tree is collapsed)
@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace labels May 1, 2020
@elaihau
Copy link
Contributor

elaihau commented Jun 14, 2020

I noticed that Theia does not have the right icon for the expand-all feature. @vince-fugnitto could you please tell me where we found all the existing icons (such as refresh and collapse) and if we will need to create CQs for it? Thank you !

@vince-fugnitto
Copy link
Member Author

I noticed that Theia does not have the right icon for the expand-all feature. @vince-fugnitto could you please tell me where we found all the existing icons (such as refresh and collapse) and if we will need to create CQs for it? Thank you !

@elaihau for the newer icons (like in vscode) we need can use codicons.
For example, the collapse-all would be:

export const COLLAPSE_ALL: Command = {
        id: 'search-in-workspace.collapse-all',
        category: SEARCH_CATEGORY,
        label: 'Collapse All',
        iconClass: 'codicon codicon-collapse-all'
};

Screenshot from 2020-06-15 08-34-21

The issue is that we do not support toggling icons (collapse / expand) as of yet: #6082

@elaihau
Copy link
Contributor

elaihau commented Jun 18, 2020

@vince-fugnitto Thank you for the explaination.

I played with your code snippet in Theia, and found the following:

  • This is what the Theia collapse-all icon looks like

Screenshot from 2020-06-17 20-21-40

  • With iconClass: 'codicon codicon-collapse-all' it looks like this

Screenshot from 2020-06-17 20-16-18

Screenshot from 2020-06-17 20-46-19

while iconClass: 'codicon codicon-bell-dot' does not display the bell-dot icon properly

Screenshot from 2020-06-17 20-23-39

Since codicons are used in quite a few places in Theia, I am thinking there should be something that determines which codicons can be displayed. Could you please point me to that place ?

@vince-fugnitto
Copy link
Member Author

@elaihau the codicons come from monaco so the reason why some might not be included can be due to the version it references. I’ll do a bit of investigation to which version it references.

@elaihau
Copy link
Contributor

elaihau commented Jun 18, 2020

@elaihau the codicons come from monaco so the reason why some might not be included can be due to the version it references. I’ll do a bit of investigation to which version it references.

Thanks.

The issue is that we do not support toggling icons (collapse / expand) as of yet

I made a poc of the icon toggling by taking advantage of the visibility:
Peek 2020-06-17 21-38

it doesn't add support to the toggling icons but ... looks like if we have the right icon we could make it work

@vince-fugnitto
Copy link
Member Author

looks like if we have the right icon we could make it work

It should work to have the following as the two icons during the toggle:

codicon codicon-collapse-all
codicon codicon-expand-all

I noticed the list of icons when viewing the following file (ex: bell-dot is not present):

  • node_modules/@theia/example-browser/lib/vs/editor/editor.main.css

@elaihau
Copy link
Contributor

elaihau commented Jun 18, 2020

looks like if we have the right icon we could make it work

It should work to have the following as the two icons during the toggle:

codicon codicon-collapse-all
codicon codicon-expand-all

sorry codicon-expand-all does not work.

Peek 2020-06-18 13-12

I will do a deep dive. if I happen to find anything I will let you know :) Thanks !

@vince-fugnitto
Copy link
Member Author

I will do a deep dive. if I happen to find anything I will let you know :) Thanks !

Correct, I don't see it in the file I referenced either.

@elaihau
Copy link
Contributor

elaihau commented Jun 18, 2020

@vince-fugnitto

made it :)
we will need a CQ to copy a few lines of code + a file from vs code.
does it worth the effort ? i mean, we could simply keep it on hold until migrating to the right version of monaco (if the icon comes from monaco)

Peek 2020-06-18 16-16

@vince-fugnitto
Copy link
Member Author

@elaihau its in the works :) #8010
I don’t think it’s urgent, it’s already a cool POC to see it is possible!

@elaihau
Copy link
Contributor

elaihau commented Jun 19, 2020

I don’t think it’s urgent, it’s already a cool POC to see it is possible!

OK I will leave the way that the icon is toggled here so someone in future could use it if s/he needs

packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts

        commands.registerCommand(SearchInWorkspaceCommands.COLLAPSE_ALL, {
            execute: w => this.withWidget(w, widget => widget.collapseAll()),
            isEnabled: w => this.withWidget(w, widget => widget.hasResultList()),
            isVisible: w => this.withWidget(w, widget => !widget.isCollapsed())
        });
        commands.registerCommand(SearchInWorkspaceCommands.EXPAND_ALL, {
            execute: w => this.withWidget(w, widget => widget.expandAll()),
            isEnabled: w => this.withWidget(w, widget => widget.hasResultList()),
            isVisible: w => this.withWidget(w, widget => widget.isCollapsed())
        });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves search in workspace issues related to the search-in-workspace
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants