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

POC refactor(ui5-select-menu, ui5-select-menu-option): removed components #8727

Closed
wants to merge 22 commits into from

Conversation

plamenivanov91
Copy link
Contributor

Related to #8461, #7887

packages/main/src/Option.ts Outdated Show resolved Hide resolved
@customElement("ui5-option")
class Option extends UI5Element implements IOption {
@customElement({
tag: "ui5-option",
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we extend CustomListItem class and at the same time we use the component of the template?

packages/main/src/Select.hbs Show resolved Hide resolved
height: var(--_ui5_list_item_dropdown_base_height);
}

:host ::part(icon) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this one is strange. you don't have icon part defined in your option.hbs file. why do we need it?

@@ -24,24 +24,570 @@
<section>
<ui5-title>Select + SelectMenu + SelectMenuOption</ui5-title><br>

<ui5-select id="sel" menu="selectOptions">
<ui5-select id="sel">
<div class="selectLabel" slot="label">
<span id="mainText">Skirt [3]</span>
<ui5-avatar id="avatar" size="XS" initials="S"></ui5-avatar>
Copy link
Contributor

Choose a reason for hiding this comment

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

some component in this test page doesn't work.

@dobrinyonkov dobrinyonkov changed the title refactor(ui5-select-menu, ui5-select-menu-option): removed components POC refactor(ui5-select-menu, ui5-select-menu-option): removed components Apr 15, 2024
@dobrinyonkov
Copy link
Contributor

Moved to #8903 as this PR is corrupted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants