Skip to content

Allow customization of the viewlets in the default Extensions view - #47766

Merged
Ramya Rao (ramya-rao-a) merged 6 commits into
microsoft:masterfrom
ryenus:separate-disabled-exts
Jun 12, 2018
Merged

Allow customization of the viewlets in the default Extensions view#47766
Ramya Rao (ramya-rao-a) merged 6 commits into
microsoft:masterfrom
ryenus:separate-disabled-exts

Conversation

@ryenus

@ryenus ryenus commented Apr 12, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

When there're lots of extensions installed, and many of them disabled, it's kinda messy to have all the enabled/disabled extensions mixed in one list.

It's more clean to separate the disabled extensions into a dedicated list view:

BeforeAfter

@msftclas

Microsoft Contribution License Agreements (msftclas) commented Apr 12, 2018 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@ryenus ryenus changed the title separate disabled extensions from installed ones separate view for disabled extensions Apr 13, 2018
@sandy081 Sandeep Somavarapu (sandy081) removed their assignment Apr 13, 2018
@ramya-rao-a

Ramya Rao (ramya-rao-a) commented Apr 16, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

In this case the Installed section would have to be renamed to Enabled.
Another way to go about this is to have 4 views shown in the default view

  • Installed
  • Recommended
  • Enabled (hidden by default)
  • Disabled (hidden by default)

To hide any of the above, user can right click on the section header and choose Hide.
To show any of the above, user can do View -> Open View and choose any number from the above 5

@ryenus

ryenus commented Apr 17, 2018

Copy link
Copy Markdown
Contributor Author

Thank you Ramya Rao (@ramya-rao-a), the first option is more close to the actual purpose, that is to change Installed to Enabled, to make it easier to go through installed extensions and enable/disable certain ones.

I've updated the PR, please let me know if rebase/squash is needed.

@ryenus

ryenus commented Apr 20, 2018

Copy link
Copy Markdown
Contributor Author

Here's the updated screenshot, with Installed changed to Enabled:

@ramya-rao-a

Ramya Rao (ramya-rao-a) commented Apr 25, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

ryenus

When trying your code, the recommended section appears first for me. This wouldnt result in a smooth transition for users.

image

This is most likely because we cache the state of the view (for each section, we cache its order in the view, height and whether it was collapsed or not). Since the new sections werent part of the cache, they show up later.

If we go with the other option then,

  • we maintain the status quo for most users
  • we give option to customize the default view to their liking
  • also, 3 sections feels a bit crowded in smaller screens if we go the current way

@ryenus

ryenus commented Apr 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

If we go with the other option ... we maintain the status quo for most users

Indeed! I mainly use the stable release, so my local dev build is too clean for me to notice the caching issue.

Let's keep the Installed section. And for Enabled/Disabled, what about make them collapsed by default?

@ryenus

ryenus commented Apr 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Now the Enabled/Disabled views are moved to the bottom,
shown as collapsed by default, and can be hidden.

@ryenus

ryenus commented Apr 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

hmm, please hold on, I tried to disable an extension, reload, then the view orders are messed up,
with Installed sits at the bottom, meanwhile Enabled/Disabled at the top, what a surprise.

@ryenus

ryenus commented Apr 26, 2018

Copy link
Copy Markdown
Contributor Author

Now view sorting works as expected, as well as customized order:

@ryenus

ryenus commented May 8, 2018

Copy link
Copy Markdown
Contributor Author

Ramya Rao (@ramya-rao-a), now the sorting issue is resolved.

The problem was viewState.order could be undefined even though viewState was valid.
I've added strict checking for viewState.order to safely fall back to viewDescriptor.order.

@ryenus ryenus mentioned this pull request May 25, 2018
Closed
ryenus added 2 commits June 8, 2018 01:15
@ryenus
add Enabled/Disabled extensions list view
c5e41be
@ryenus
extract viewOrder calculation logic
d5f52f8 
and return Number.MAX_VALUE instead of Number.POSITIVE_INFINITY for
views end up with undefined order, when comparing two views like this,
we get Number.MAX_VALUE - Number.MAX_VALUE = 0, rather than NaN
caused by infinity arithmetic.
ramya-rao-a
Ramya Rao (ramya-rao-a) suggested changes Jun 8, 2018
View reviewed changes

@ramya-rao-a Ramya Rao (ramya-rao-a) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks ryenus, the PR is looking good!
I have left a few comments, please do take a look.

Sandeep Somavarapu (@sandy081) As part of this feature, we would want to enable the canToggleVisibility on the Installed section in the extensions view. If someone chooses to have just the Enabled section or Enabled and Disabled section separately, then it doesnt make sense to not let them hide the "Installed" section.

Comment thread src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts Outdated
Comment thread src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts
Comment thread src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts Outdated
Comment thread src/vs/workbench/parts/extensions/electron-browser/extensionsViewlet.ts
Comment thread src/vs/workbench/browser/parts/views/views.ts
@ramya-rao-a Ramya Rao (ramya-rao-a) changed the title separate view for disabled extensions Allow customization of the viewlets in the default Extensions view Jun 8, 2018
@ramya-rao-a Ramya Rao (ramya-rao-a) added this to the June 2018 milestone Jun 8, 2018
ramya-rao-a
Ramya Rao (ramya-rao-a) reviewed Jun 9, 2018
View reviewed changes
Comment thread src/vs/workbench/parts/extensions/electron-browser/extensionsViews.ts Outdated
ramya-rao-a
Ramya Rao (ramya-rao-a) approved these changes Jun 10, 2018
View reviewed changes

@ramya-rao-a Ramya Rao (ramya-rao-a) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes look good now ryenus
Just waiting on Sandeep Somavarapu (@sandy081) on his thoughts for the changes to the compareViewDescriptors and making the Installed section hide-able after which we can merge the changes

@ramya-rao-a
Ramya Rao (ramya-rao-a) merged commit e8fc894 into microsoft:master Jun 12, 2018
@ryenus

ryenus commented Jun 12, 2018

Copy link
Copy Markdown
Contributor Author

Ramya Rao (@ramya-rao-a) I'm really glad to see this is now merged, thank you so much for all the help!

@ramya-rao-a

Ramya Rao (ramya-rao-a) commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

Its our pleasure ryenus, thanks for all your work.

Happy Coding!

@ramya-rao-a Ramya Rao (ramya-rao-a) mentioned this pull request Jun 12, 2018
Closed
@ryenus
ryenus deleted the separate-disabled-exts branch June 13, 2018 17:05
@ramya-rao-a Ramya Rao (ramya-rao-a) mentioned this pull request Jun 15, 2018
Closed
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandy081 Sandeep Somavarapu (sandy081) sandy081 left review comments
+1 more reviewer
@ramya-rao-a Ramya Rao (ramya-rao-a) ramya-rao-a approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ramya-rao-a Ramya Rao (ramya-rao-a)

Labels

None yet

Projects

None yet

Milestone

June 2018

Development

Successfully merging this pull request may close these issues.

4 participants

@ryenus @msftclas @ramya-rao-a @sandy081