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

refactor tenant wide extensions for app catalog toggle #409

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

ervingayle
Copy link
Contributor

🎯 Aim

This PR refactors the tenant wide extensions logic to control the display of the app catalogs and the expansion of those if the setting is enabled

📷 Result

vscode-viva-itemid-325-a
vscode-viva-itemid-325-b
vscode-viva-itemid-325-c
vscode-viva-itemid-325-d

✅ What was done

clarify what was done and what still needs to be finished ex. [Remove this line]

  • Updated setting title and description
  • Removed default show/hide for tenant extensions
  • Added show hide for appcatalogs and deployed apps including the toggle icon

🔗 Related issue

Closes: #325

@Adam-it Adam-it self-assigned this Feb 7, 2025
Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@ervingayle Looks like we are on the right track 👍👏 You Rock 🤩.

I left a few comments and I noticed we overlooked updating the site level app catalogs as well based on the setting
image

package.json Outdated
@@ -190,10 +190,10 @@
"description": "Show the service health incidents in the account view."
},
"spfx-toolkit.showTenantWideExtensions": {
Copy link
Member

Choose a reason for hiding this comment

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

it would be best if we also modify the setting identifier name from showTenantWideExtensions to showAppsInAppCatalogs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Adam-it . Thanks for your feedback. For some reason I thought you did not want to rename the setting. I made the change.

@Adam-it Adam-it marked this pull request as draft February 16, 2025 00:04
@ervingayle ervingayle marked this pull request as ready for review February 16, 2025 04:12
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.

💡 [Feature]: Remove or Refactor the show/hide tenant wide extension list
2 participants