Skip to content

Topic style fixes #199

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.

Sign up for GitHub

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

Merged
coryrylan merged 2 commits into from
Jul 30, 2026
Merged

Topic style fixes #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
596517a
fix(core): drawer positioning
coryrylan Jul 30, 2026
e928e36
fix(core): update select size handling and improve layout
coryrylan Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions projects/core/.visual/pagination.dark.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions projects/core/.visual/pagination.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions projects/core/.visual/select.dark.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions projects/core/.visual/select.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions projects/core/src/drawer/drawer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,17 @@ nve-icon-button {
}

:host([position='left']) {
inset: auto auto auto 0;
inset: auto auto auto 0 !important;
}

:host([position='right']) {
inset: auto 0 auto auto;
inset: auto 0 auto auto !important;
}

:host([position='top']) {
inset: 0 auto auto;
inset: 0 auto auto !important;
}

:host([position='bottom']) {
inset: auto auto 0;
inset: auto auto 0 !important;
}
13 changes: 13 additions & 0 deletions projects/core/src/drawer/drawer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,19 @@ describe(Drawer.metadata.tag, () => {
expect(element.getAttribute('position')).toBe('right');
});

it.each(['right', 'bottom'] as const)('should position at the viewport %s edge', async position => {
element.position = position;
await elementIsStable(element);

const open = untilEvent(element, 'open');
element.showPopover();
await open;

const edge = position === 'right' ? 'right' : 'bottom';
const viewportEdge = position === 'right' ? window.innerWidth : window.innerHeight;
expect(element.getBoundingClientRect()[edge]).toBe(viewportEdge);
});

it('should reflect modal attribute to DOM', async () => {
expect(element.hasAttribute('modal')).toBe(false);
element.modal = true;
Expand Down
24 changes: 15 additions & 9 deletions projects/core/src/select/select.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@
--border-radius: var(--nve-ref-border-radius-sm);
--border: var(--nve-ref-border-width-md) solid transparent;
--border-bottom: var(--border);
--gap: var(--nve-ref-space-xxs);
--cursor: pointer;
--font-weight: normal;
--text-align: start;
--scroll-height: 50vh;
--_input-background: var(--background);
--_icon-color: var(--color);
--width: 100%;
--control-width: var(--width);
--max-width: 100%;
--min-width: fit-content;
--_input-background: var(--background);
--_icon-color: var(--color);
--_gap: var(--gap);
--size: 1;
--_row-height: calc(var(--height) * var(--size) + var(--gap) * (var(--size) - 1));
--_height: calc(var(--_row-height) + var(--nve-ref-border-width-md) * 2);
contain: initial;
width: var(--width);
max-width: var(--max-width);
Expand All @@ -35,7 +40,11 @@
}

[input] {
height: var(--height);
scroll-behavior: smooth;
scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%)

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use lowercase currentcolor to satisfy Stylelint.

value-keyword-case reports this declaration. CSS behavior is unchanged, but the current spelling can fail the style check.

Proposed fix 
-  scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%)
+  scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentcolor 20%)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%)
scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentcolor 20%)
🧰 Tools
🪛 Stylelint (17.14.1)

[error] 44-44: Expected "currentColor" to be "currentcolor" (value-keyword-case)

(value-keyword-case)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@projects/core/src/select/select.css` at line 44, Update the scrollbar-color
declaration in the select stylesheet to use lowercase currentcolor in the
color-mix expression, preserving the existing value and behavior.

Source: Linters/SAST tools

var(--nve-sys-scrollbar-track-color);
scrollbar-width: var(--nve-sys-scrollbar-width);
height: var(--_height);
background: var(--background);
border: var(--border);
border-bottom: var(--border-bottom);
Expand Down Expand Up @@ -64,7 +73,7 @@
cursor: var(--cursor) !important;
font-weight: var(--font-weight) !important;
text-align: var(--text-align) !important;
height: var(--height) !important;
height: var(--_height) !important;
width: 100% !important;
border: 0 !important;
background: none !important;
Expand Down Expand Up @@ -94,7 +103,7 @@
:host(:state(multiple)) .tags {
width: fit-content;
display: flex;
gap: var(--nve-ref-space-xxs);
gap: var(--gap);
}

:host(:state(multiple)) .tags-label {
Expand Down Expand Up @@ -130,10 +139,6 @@
opacity: 0 !important;
}

:host(:state(size)) {
--height: calc(var(--nve-ref-size-800) * var(--size));
}

:host(:state(size)) [input] {
overflow: auto;
display: block;
Expand Down Expand Up @@ -184,6 +189,7 @@ nve-dropdown {

nve-menu {
--max-height: var(--scroll-height);
--gap: var(--_gap);
}

nve-menu-item {
Expand Down
34 changes: 33 additions & 1 deletion projects/core/src/select/select.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,39 @@ describe(`${Select.metadata.tag}: size`, () => {
});

it('should set --size property', () => {
expect(getComputedStyle(element).getPropertyValue('--size')).toBe('3.75'); // size (3) + 0.75 buffer
expect(getComputedStyle(element).getPropertyValue('--size')).toBe('3');
});

it('should show all rows without overflow', async () => {
const select = fixture.querySelector('select') as HTMLSelectElement;
for (let optionNumber = 6; optionNumber <= 10; optionNumber++) {
const option = document.createElement('option');
option.value = `${optionNumber}`;
option.textContent = `Option ${optionNumber}`;
select.append(option);
}
select.size = 10;
await elementIsStable(element);

const input = element.shadowRoot.querySelector<HTMLElement>('[input]') as HTMLElement;
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag) as NodeListOf<MenuItem>;
const inputBottom =
input.getBoundingClientRect().bottom - Number.parseFloat(getComputedStyle(input).borderBottomWidth);
const lastItemBottom = items[items.length - 1].getBoundingClientRect().bottom;

expect(input.scrollTop).toBe(0);
expect(lastItemBottom).toBeCloseTo(inputBottom, 5);
});

it('should not leave blank space after the visible rows', () => {
const input = element.shadowRoot.querySelector<HTMLElement>('[input]');
const items = element.shadowRoot.querySelectorAll<MenuItem>(MenuItem.metadata.tag);
const inputBottom =
input.getBoundingClientRect().bottom - Number.parseFloat(getComputedStyle(input).borderBottomWidth);
const lastVisibleItemBottom = items[2].getBoundingClientRect().bottom;

expect(input.scrollTop).toBe(0);
expect(lastVisibleItemBottom).toBeCloseTo(inputBottom, 5);
});

it('should not render tags when using multiple with size', async () => {
Expand Down
2 changes: 1 addition & 1 deletion projects/core/src/select/select.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ export class Select extends Control {
super.updated(props);
if (this.#select?.size && this.#select?.size !== 0) {
this._internals.states.add('size');
this.style.setProperty('--size', `${this.#select?.size + 0.75}`);
this.style.setProperty('--size', `${this.#select.size}`);
} else {
this._internals.states.delete('size');
this.style.removeProperty('--size');
Expand Down