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: banner-context #44

Merged
merged 12 commits into from
Jul 8, 2024
Merged

feat: banner-context #44

merged 12 commits into from
Jul 8, 2024

Conversation

PabloReszczynski
Copy link
Collaborator

@PabloReszczynski PabloReszczynski commented Jul 3, 2024

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:

<topsort-banner context="true" id="an-example-id" width="800" height="400">
  <topsort-banner-slot rank="1"></topsort-banner-slot>
  <topsort-banner-slot rank="2"></topsort-banner-slot>
</topsort-banner>

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.

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>">
Copy link
Member

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

Copy link
Collaborator Author

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

Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

index.html Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@PabloReszczynski PabloReszczynski merged commit 15129e5 into main Jul 8, 2024
4 checks passed
@PabloReszczynski PabloReszczynski deleted the context branch July 8, 2024 17:28
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.

2 participants