-
Notifications
You must be signed in to change notification settings - Fork 1
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: banner-context #44
Conversation
README.md
Outdated
`topsort-banner-context` takes the same properties as the regular `topsort-banner` element. | ||
|
||
```html | ||
<topsort-banner-context width="600" height="400" id="<your slot id>"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep the original <topsort-banner>
element instead of making a new one, and then look for a sub-tag (similar to what you have; perhaps <topsort-winner rank="1">
? The api feels easier to understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally I would use the same element and then pass different attrs depending on context. But Idk how to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking that we'd iterate the sub elements and look for <topsort-winner rank="1">
before deciding what logic to apply?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Easier said than done, as there is no easy way for a component to check its children if they have any, as everything is built in an asynchronous manner. So at some point in time the banner could have no children, and then it does. Same goes with attributes: Components attributes are undefined
on construction and then defined after it is mounted in the Shadow DOM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some more digging and experimentation I manage to do it with one component. Tho is more complicated this way, at least we don't have to maintain 3 components.
Introduce 1 new components:
topsort-banner-slot
. This provides a way to show multiple banners for one slotId and running one auction.Now
topsort-banner
can be used in two ways: Rendering one banner, or multiple.Example use case of multiple banners:
Also update the index.html demo page to showcase all the banner elements.
And refactor the main logic of the main banner as a mixin so it is reusable between the different components.