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

Controller: @phase2/outline-controller-slot-manager #389

Open
wants to merge 9 commits into
base: next
Choose a base branch
from

Conversation

bardleb
Copy link

@bardleb bardleb commented May 16, 2023

Description

Separated Slot Controller into its own package and renamed as Slot Manager

Summary by CodeRabbit

  • New Features
    • Introduced the SlotManager for enhanced slot management within the shadow DOM, including cloning slots, adding annotations, and event dispatching between shadow and light DOM slots.
  • Documentation
    • Updated the README.md for the SlotManager with usage and feature documentation.
  • Refactor
    • Exported the SlotManager class from src/slot-manager in a new index.ts file.
    • Introduced a new slot-manager.ts file with the SlotManager class for managing slots in Lit web components.

@changeset-bot
Copy link

changeset-bot bot commented May 16, 2023

⚠️ No Changeset found

Latest commit: cd94916

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented May 16, 2023

Deploy Preview for outlinejs ready!

Name Link
🔨 Latest commit cd94916
🔍 Latest deploy log https://app.netlify.com/sites/outlinejs/deploys/65c29bed6edbb0000808ecee
😎 Deploy Preview https://deploy-preview-389--outlinejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@shaal shaal changed the title Feature/slot controller package Feature/slot-manager package (previously called slots-controller) May 22, 2023
@himerus himerus added Type: Controller Suggestion Use this label for any issue that is a recommendation for a specific type of controller. CANNOT MERGE Temporary flag for a PR that has failing tests, broken pipelines, etc. labels May 22, 2023
@himerus himerus removed the CANNOT MERGE Temporary flag for a PR that has failing tests, broken pipelines, etc. label May 22, 2023
@himerus himerus changed the title Feature/slot-manager package (previously called slots-controller) Controller: @phase2/outline-controller-slot-manager May 22, 2023
@himerus himerus self-assigned this May 22, 2023
Copy link
Author

Choose a reason for hiding this comment

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

Please add code examples for usage with ShadowDOM, LightDOM and how to handle a single unnamed (default) slot.

@himerus himerus added the Held for Review Held for review. Considered as a Draft/POC. label Jul 25, 2023
@himerus
Copy link
Contributor

himerus commented Jul 25, 2023

This is marked Held for Review until further notice.
cc @bardleb @shaal

Copy link
Contributor

@himerus himerus left a comment

Choose a reason for hiding this comment

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

At a core level, I'd like to see/discuss this being moved into TWO controllers.

  • Controller 1: SlotManager - Has all sorts of cool slot tools for ALL components that have slots.
    • Testing if a slot exists
    • Conditional rendering a slot
  • Controller 2: SlotShifter (lol) - This one should, and I know this is a pain be a standalone piece that can be additive to the SlotManager.
    • Can this EXTEND the SlotManager to add in the additional functionality related to managing slot movement/event delegation/etc??

@glitchgirl
Copy link
Contributor

TODO:

  • review this code and simplify to only contain shadow dom tools

Copy link

coderabbitai bot commented Feb 6, 2024

Walkthrough

The update introduces the SlotManager, a utility class for managing slots in Lit web components. It provides methods for interacting with and manipulating slots, including retrieving slotted nodes, checking slot existence, identifying default slot text and elements, and conditionally rendering slots with wrappers and classes.

Changes

File(s) Change Summary
.../slot-manager/README.md Introduced documentation for the SlotManager, detailing its purpose and capabilities in managing slots within Lit web components.
.../slot-manager/index.ts, src/slot-manager.ts Added index.ts to export the SlotManager class and implemented slot-manager.ts with methods for slot management in Lit web components.

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 9

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 330e286 and ff89682.
Files ignored due to path filters (2)
  • packages/controllers/slot-manager/package.json is excluded by: !**/*.json
  • packages/controllers/slot-manager/tsconfig.build.json is excluded by: !**/*.json
Files selected for processing (3)
  • packages/controllers/slot-manager/README.md (1 hunks)
  • packages/controllers/slot-manager/index.ts (1 hunks)
  • packages/controllers/slot-manager/src/slot-manager.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/controllers/slot-manager/index.ts
Additional comments: 1
packages/controllers/slot-manager/src/slot-manager.ts (1)
  • 373-400: The conditionalSlot method's implementation is clear, but ensure that the use of ifDefined and classMap from lit is optimal for the intended use cases.

packages/controllers/slot-manager/README.md Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
packages/controllers/slot-manager/src/slot-manager.ts Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ff89682 and a208ab9.
Files selected for processing (1)
  • packages/controllers/slot-manager/src/slot-manager.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/controllers/slot-manager/src/slot-manager.ts

@himerus
Copy link
Contributor

himerus commented Feb 6, 2024

@coderabbitai review

@himerus himerus self-requested a review February 6, 2024 20:24
@himerus himerus removed the Held for Review Held for review. Considered as a Draft/POC. label Feb 6, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a208ab9 and cd94916.
Files ignored due to path filters (1)
  • packages/controllers/slot-manager/package.json is excluded by: !**/*.json
Files selected for processing (2)
  • packages/controllers/slot-manager/README.md (1 hunks)
  • packages/controllers/slot-manager/src/slot-manager.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • packages/controllers/slot-manager/README.md
  • packages/controllers/slot-manager/src/slot-manager.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for Review Type: Controller Suggestion Use this label for any issue that is a recommendation for a specific type of controller.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants