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

File nesting: Improve file icon rendering for folder less themes #141428

Closed
aeschli opened this issue Jan 25, 2022 · 16 comments
Closed

File nesting: Improve file icon rendering for folder less themes #141428

aeschli opened this issue Jan 25, 2022 · 16 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues polish Cleanup and polish issue verified Verification succeeded
Milestone

Comments

@aeschli
Copy link
Contributor

aeschli commented Jan 25, 2022

Testing #141354

  • set "explorer.experimental.fileNesting.enabled": true
  • open a folder with foo.js and foo.ts
  • use the Seti file icon theme
    image

Theres a misalignment as the Seti theme doesn't expect that a file can be a root node.
We probably have to hide the icon of foo.ts

@JacksonKearl
Copy link
Contributor

cc @misolori any ideas on how this could look? I like how the icon lets me know it's a file, but agree the alignment is weird.

@JacksonKearl JacksonKearl added the file-explorer Explorer widget issues label Jan 26, 2022
@miguelsolorio
Copy link
Contributor

I'd probably hide the file icon for the parent folder, even though it isn't ideal but you can't get alignment and folder icon unless we add a folder icon for seti icons:

CleanShot 2022-01-25 at 20 39 51@2x

@chrisdias
Copy link
Member

chrisdias commented Jan 28, 2022

with the minimal file icon theme, the nested files are indented nicely

image

As compared to seti...

image

@JacksonKearl JacksonKearl added this to the February 2022 milestone Jan 28, 2022
@JacksonKearl
Copy link
Contributor

I'll try to find a way to make Seti work well next iteration, I'd rather not lose the quick identifiability the icons provide but agree the current alignment is very odd

@JacksonKearl JacksonKearl added bug Issue identified by VS Code Team member as probable bug polish Cleanup and polish issue labels Jan 28, 2022
@aeschli
Copy link
Contributor Author

aeschli commented Jan 31, 2022

@JacksonKearl Happy to help and review.
align-icons-and-twisties is the css rule that is set when a file icon theme only defines file icons but no folder icons.

@aeschli
Copy link
Contributor Author

aeschli commented Jan 31, 2022

There are also file icon themes that disable the twisties. Instead, they provide different folder icons for expanded and collapsed folders.
An example is the Ayu theme:
image

Again such themes don't work well with file nesting
Peek 2022-01-31 11-07

I can open a different issue for that.

@yume-chan
Copy link
Contributor

There are also file icon themes that disable the twisties. Instead, they provide different folder icons for expanded and collapsed folders.

#141359

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Feb 2, 2022

Another idea that came from the ux sync was aligning the other files to the same indentation in the case where there is no folder icon (like seti) and since they are not sibilings they should be on the same indentation:

Since we already make adjustments when either there is a folder chevron or folder icon, we should be able to make adjustment based on these options + file nesting is enabled

@babakfp
Copy link

babakfp commented Feb 21, 2022

Hi @JacksonKearl
I see you are working on this feature. What do you think about a config (for each pattern) that disables the ability to hide the nested files (child files)? So they can always be visible and accessible. I see that it can be a useful feature to add. For example, this is what it is right now:
Screenshot 2022-02-21 131244
... and this is how I want it to be:
Screenshot 2022-02-21 131207


Also, an option for each pattern to hide the child file icon if its name was equal to the parent file name or something like that.

@JacksonKearl
Copy link
Contributor

@babakfp by default all nests are shown expanded, I don't see why we'd want to remove the option of collapsing them?

@babakfp
Copy link

babakfp commented Mar 2, 2022

Hi
Thank you for your response

by default all nests are shown expanded, I don't see why we'd want to remove the option of collapsing them?

It's because of this problem and reasons👇
#141359

Thank you again. A saw a lot of issues assigned to you, here is a coffee🍵😅🌸

@JacksonKearl

@JacksonKearl
Copy link
Contributor

#141359 is definitely a bug, but it only appears when the file icon theme has disabled folder chevrons. I will be fixing that this iteration by rendering nests with chevrons regardless of whether they're rendered with folders.

JacksonKearl pushed a commit that referenced this issue Mar 9, 2022
- Force nests to render with a twistie
- Resolve alignment issues by hiding nested parent icon to match folders
Fixes #142938, #141428, #141359
@JacksonKearl
Copy link
Contributor

We now hide the icon of a nest parent if the folders do not have chevrons or icons

@babakfp
Copy link

babakfp commented Mar 9, 2022

I will be fixing that this iteration by rendering nests with chevrons regardless of whether they're rendered with folders.

But it's not the same. There is an option to hide the 'angle down/arrow down' icon, so it means that there was/is a need for this option/this functionality. By doing what you said, you are destroying that functionality!

We now hide the icon of a nest parent if the folders do not have chevrons or icons

What do you mean is that this (TS) icon is going to be hidden?:

150968856-1302b2a9-def5-408a-aa2f-38e9e241532b

It's not a good thing to do if it's not optional for each nest. (Maybe I misunderstood what you meant)

@JacksonKearl

@JacksonKearl
Copy link
Contributor

That is what I have now, though I can see the sense in doing this instead.

@babakfp
Copy link

babakfp commented Mar 9, 2022

That is what I have now, though I can see the sense in doing #141428 (comment) instead.

Yeah, in my opinion, this one is better than removing the icon.

It's also useful to only group files together(for ordering porpuses) instead of nesting them under each other.

@aeschli aeschli added the verified Verification succeeded label Mar 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues polish Cleanup and polish issue verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants