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

feat: add MainAppSlot for chatbot plugin #1320

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Conversation

awais-ansari
Copy link

INF-1573

Description

  • This PR adds MainAppSlot, allowing a plugin at the root level. For example the chatbot plugin.
  • This PR removes the automatic use of the Zendesk component from the App level. The Zendesk logic still lives in the src/common-components directory.

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.57%. Comparing base (b41fca3) to head (a45503f).
Report is 31 commits behind head on 2u-main.

Additional details and impacted files
@@             Coverage Diff             @@
##           2u-main    #1320      +/-   ##
===========================================
+ Coverage    86.56%   86.57%   +0.01%     
===========================================
  Files          133      134       +1     
  Lines         2411     2413       +2     
  Branches       669      668       -1     
===========================================
+ Hits          2087     2089       +2     
  Misses         311      311              
  Partials        13       13              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jsnwesson
Copy link

This looks great so far! One thing that I would request is that in your /plugin-slots directory if you can follow the style of documentation that is in Learner Dashboard and Learning's /plugin-slots directories.
Both also include a section in the main README that plugin slots exist and points to where they can be found. That would be useful to include here as well.

@jsnwesson
Copy link

Oh that's riiight, this is only a PR that merges into the forked 2u-main branch. Disregard my request unless you think it'll be useful for your own team. If there's a future case where this PR will eventually be pushed up to the master branch, then I think that documentation will have a stronger ask.

@awais-ansari awais-ansari merged commit 47b0501 into 2u-main Sep 25, 2024
7 checks passed
@awais-ansari awais-ansari deleted the aansari/INF-1573 branch September 25, 2024 08:58
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.

5 participants