Skip to content

attachmentList changes - #897

Merged
timea-solid merged 6 commits into
stagingfrom
improveFriends
Oct 20, 2026
Merged

timea-solid merged 6 commits into
stagingfrom
improveFriends

Conversation

@timea-solid

Copy link
Copy Markdown
Member

This is improving the attachmentLidt function by adding an optional parameter (it does not chnage the API) which determines in which container/div teh delete pop up appears.
This PR also changes the go-to-icon to a new lucide icon. The side effect is that in the file explorer the icon will look like:
Screenshot 2026-10-19 at 21 12 40
This is how it looks on the friends list now too.
image

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Closing externally focused confirmations needs focus restoration so users retain their position in the list.

1 open issue
What changed in this PR

Adds configurable placement for attachment deletion confirmations while preserving default row-local behavior.

Changes:

  • Threads the optional confirmation container through widget helpers.
  • Replaces default navigation images with Lucide icons.
  • Adds combobox focus styling, documentation, and regression tests.
File Description
test/​unit/​widgets/​buttons/​iconLink.test.ts Tests Lucide rendering and custom icons.
test/​unit/​widgets/​buttons.test.ts Tests confirmation placement and handlers.
src/​widgets/​buttons/​iconLinks.ts Uses Lucide navigation icons by default.
src/​widgets/​buttons.ts Supports external confirmation containers.
src/​components/​combobox/​Combobox.styles.css Adds visible keyboard focus styling.
README.md Documents icons and confirmation options.

Review effort: Balanced

Comment thread src/widgets/buttons.ts
padding-right: 40px;
width: 100%;

/* Global focus styles don't reach into the shadow DOM, so mirror them here. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think these are the way we should do the styles anymore I might be wrong but I think this is the way we did them in mashlib but now everything is in themes.css

so for instance i think they should be
outline: 2px solid var(--solid-ui-color-primary, #7C4DFF). (only --focus-indicator-width and this --color-primary if you wanted style configuration from the caller, and with that you should add them at the top under :host if you want to do that.
for
box-shadow: 0 0 0 1px #F8F9FB, 0 0 0 4px rgba(124, 77, 255, 0.2); maybe just that for now, otherwise you can also add to themes.css a solid-ui-color-{color}-{level} for this color.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we need a bigger style redo in most panes. Later.

@SharonStrats SharonStrats left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I just had the one comment about the styles in combobox, but everything else looks great!

@timea-solid
timea-solid merged commit 89504e0 into staging Oct 20, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in SolidOS NLNet UI Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants