Skip to content

Add/Edit Bookmark Dialog - #278

Open
F3l1x1vo wants to merge 19 commits into
mainfrom
feat/app-bookmark-dialog
Open

Add/Edit Bookmark Dialog#278
F3l1x1vo wants to merge 19 commits into
mainfrom
feat/app-bookmark-dialog

Conversation

@F3l1x1vo

@F3l1x1vo F3l1x1vo commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

This PR adds a bookmark modal for adding and editing bookmarks. Currently, they are saved in localStorage (not a database like we hoped. The database is a v1 feature of the S3 Storage Browser and not yet available on the parent branch). Migrating these bookmarks to the database is an issue for later (#277).

The bookmarks are currently shown in a section on the dashboard but we will change the display of these as well (#271)


Closes #270

@F3l1x1vo
F3l1x1vo requested a review from dklOrdix July 31, 2026 07:26
@F3l1x1vo F3l1x1vo self-assigned this Jul 31, 2026
@F3l1x1vo
F3l1x1vo changed the base branch from feat/iframe-modularization to main July 31, 2026 08:11
@F3l1x1vo
F3l1x1vo force-pushed the feat/app-bookmark-dialog branch from c622e28 to fbfa6e6 Compare July 31, 2026 08:17
F3l1x1vo and others added 14 commits September 2, 2026 07:23
* Add OPA client for admin rights checking

- New OPA client module (src/lib/server/opa.ts) using plain fetch
- Separate handleOpaAdmin middleware hook (fail-closed)
- Admin status exposed to client via event.locals.isAdmin and $page.data
- Prometheus metrics: opa_request_duration_seconds + opa_request_total
- Helm chart for OPA 1.16.2 (dev/opa/)
- Sample Rego policy with admin email domain check
- Dev setup: alice is admin (alice@admin.example.com)
- Configurable via STACKABLE_COCKPIT_OPA_ENABLED/URL/TIMEOUT env vars

* Replace custom OPA fetch client with @open-policy-agent/opa SDK

Uses OPAClient.evaluate() with fromResult callback instead of raw fetch.
Same external interface (checkAdmin), same error handling (fail-closed).

* fix YAML format

* fix YAML format

* fix YAML format

* fix: handle bare boolean responses from OPA admin check

The fromResult callback expected { admin: true } but OPA may return a
bare boolean. Now handles both formats.

* chore: remove merge artifact feature flags

Remove unused archivePreviewMaxBytes and commented-out env vars
(INFINITE_SCROLL, CUT_COPY, PASTE, RENAME, MOVE) that have no
corresponding code.

---------

Co-authored-by: F3l1x1vo <fli@ordix.de>
@F3l1x1vo
F3l1x1vo force-pushed the feat/app-bookmark-dialog branch from 286a34a to 7a7d76c Compare September 2, 2026 05:23
F3l1x1vo and others added 3 commits September 2, 2026 07:47
* #270 add bookmark dialog

* add edit bookmarks dialog

* fix cancel editing bookmark

* fix darkmode for icons, cursor pointer over boxes

* update product icons to use colored versions where possible

* scaled icons down to 64x64

* Revert "scaled icons down to 64x64"

This reverts commit 740391c.

* revert image downscale, use svelte enhanced image component to downscale images

* #271 overhaul dashboard layout design

* fix unit test

* Add OPA client for admin rights checking (#276)

* Add OPA client for admin rights checking

- New OPA client module (src/lib/server/opa.ts) using plain fetch
- Separate handleOpaAdmin middleware hook (fail-closed)
- Admin status exposed to client via event.locals.isAdmin and $page.data
- Prometheus metrics: opa_request_duration_seconds + opa_request_total
- Helm chart for OPA 1.16.2 (dev/opa/)
- Sample Rego policy with admin email domain check
- Dev setup: alice is admin (alice@admin.example.com)
- Configurable via STACKABLE_COCKPIT_OPA_ENABLED/URL/TIMEOUT env vars

* Replace custom OPA fetch client with @open-policy-agent/opa SDK

Uses OPAClient.evaluate() with fromResult callback instead of raw fetch.
Same external interface (checkAdmin), same error handling (fail-closed).

* fix YAML format

* fix YAML format

* fix YAML format

* fix: handle bare boolean responses from OPA admin check

The fromResult callback expected { admin: true } but OPA may return a
bare boolean. Now handles both formats.

* chore: remove merge artifact feature flags

Remove unused archivePreviewMaxBytes and commented-out env vars
(INFINITE_SCROLL, CUT_COPY, PASTE, RENAME, MOVE) that have no
corresponding code.

---------

Co-authored-by: F3l1x1vo <fli@ordix.de>

* chore: dedupe packages

* #270 add admin check to pinning, add OPA tests

* fix unit test

* #270 show pin for all only to admins

* fix unit test

* pull from feat/app-bookmark-dialog

* pull from feat/app-bookmark-dialog

---------

Co-authored-by: dklOrdix <dkl@ordix.de>
… as tools inside the cockpit (#283)

* #270 add bookmark dialog

* add edit bookmarks dialog

* fix cancel editing bookmark

* fix darkmode for icons, cursor pointer over boxes

* update product icons to use colored versions where possible

* scaled icons down to 64x64

* Revert "scaled icons down to 64x64"

This reverts commit 740391c.

* revert image downscale, use svelte enhanced image component to downscale images

* #275 allow users to pin bookmarks

* #275 #280 add bookmarks to sidebar navigation, show external apps in cockpit

* fix unit test

* Add OPA client for admin rights checking (#276)

* Add OPA client for admin rights checking

- New OPA client module (src/lib/server/opa.ts) using plain fetch
- Separate handleOpaAdmin middleware hook (fail-closed)
- Admin status exposed to client via event.locals.isAdmin and $page.data
- Prometheus metrics: opa_request_duration_seconds + opa_request_total
- Helm chart for OPA 1.16.2 (dev/opa/)
- Sample Rego policy with admin email domain check
- Dev setup: alice is admin (alice@admin.example.com)
- Configurable via STACKABLE_COCKPIT_OPA_ENABLED/URL/TIMEOUT env vars

* Replace custom OPA fetch client with @open-policy-agent/opa SDK

Uses OPAClient.evaluate() with fromResult callback instead of raw fetch.
Same external interface (checkAdmin), same error handling (fail-closed).

* fix YAML format

* fix YAML format

* fix YAML format

* fix: handle bare boolean responses from OPA admin check

The fromResult callback expected { admin: true } but OPA may return a
bare boolean. Now handles both formats.

* chore: remove merge artifact feature flags

Remove unused archivePreviewMaxBytes and commented-out env vars
(INFINITE_SCROLL, CUT_COPY, PASTE, RENAME, MOVE) that have no
corresponding code.

---------

Co-authored-by: F3l1x1vo <fli@ordix.de>

* fix unit test

* Add OPA client for admin rights checking (#276)

* Add OPA client for admin rights checking

- New OPA client module (src/lib/server/opa.ts) using plain fetch
- Separate handleOpaAdmin middleware hook (fail-closed)
- Admin status exposed to client via event.locals.isAdmin and $page.data
- Prometheus metrics: opa_request_duration_seconds + opa_request_total
- Helm chart for OPA 1.16.2 (dev/opa/)
- Sample Rego policy with admin email domain check
- Dev setup: alice is admin (alice@admin.example.com)
- Configurable via STACKABLE_COCKPIT_OPA_ENABLED/URL/TIMEOUT env vars

* Replace custom OPA fetch client with @open-policy-agent/opa SDK

Uses OPAClient.evaluate() with fromResult callback instead of raw fetch.
Same external interface (checkAdmin), same error handling (fail-closed).

* fix YAML format

* fix YAML format

* fix YAML format

* fix: handle bare boolean responses from OPA admin check

The fromResult callback expected { admin: true } but OPA may return a
bare boolean. Now handles both formats.

* chore: remove merge artifact feature flags

Remove unused archivePreviewMaxBytes and commented-out env vars
(INFINITE_SCROLL, CUT_COPY, PASTE, RENAME, MOVE) that have no
corresponding code.

---------

Co-authored-by: F3l1x1vo <fli@ordix.de>

* fix auth port, implement open in new tab option, fix custom link resolve

* #270 add admin check to pinning, add OPA tests

* fix unit test

* #270 show pin for all only to admins

* fix unit test

* pull from feat/app-bookmark-dialog

---------

Co-authored-by: dklOrdix <dkl@ordix.de>
# Conflicts:
#	TECH_DEBT.md
#	src/lib/server/auth.ts
@stackable-cla

stackable-cla Bot commented Sep 10, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ F3l1x1vo
❌ dklOrdix
You have signed the CLA already but the status is still pending? Let us recheck it.

@Bohreromir
Bohreromir self-requested a review September 11, 2026 07:41

@Bohreromir Bohreromir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unless I do miss something the whole PinForEveryone does not work as its only stored in LocalStorage. Opa could then be removed too.

I havent yet read the whole thing, but I think this is enough for the meantime.

Feel free to ask for any clarifications.

{#if product.logo}
<enhanced:img
src={product.logo}
alt=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please add an alt

Comment thread vite.config.ts
export default defineConfig({
plugins: [
tailwindcss(),
enhancedImages(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we use enhanced img for the already existing imgs too?

Comment thread .env.example
# Feature flags
# STACKABLE_COCKPIT_COMPLETION_ENABLED=false # Disable SQL editor code completion (default: true)
# STACKABLE_COCKPIT_STORAGE_BROWSER_ENABLED=true # Enable S3/HDFS file browser (default: false)
# PUBLIC_STACKABLE_COCKPIT_UPLOAD_CONCURRENCY=3 # Maximum number of concurrent file uploads (default: 3)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

did you mean to remove this?

Comment thread .env.example
Comment on lines +47 to +48
# OPA (Open Policy Agent) — admin rights checking
# STACKABLE_COCKPIT_OPA_ENABLED=true # Enable OPA admin checks (default: false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small contradiciton here. i guess most users would copy the .env.example to craft there own. If they do, the default is true while comment says false

if (typeof bookmark.id === 'string' && isValidBookmarkId(bookmark.id)) {
return bookmarkWithoutOpenIn;
}
migrated = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Whats being migrated here? Bookmarks do not exist outside of this PR as far as i know

addModalOpen = true;
}

function getProduct(productId: string) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this fn exists twice and could be a shared helper in products.ts

}
}

function handleLogoError(e: Event) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also repeated

productId: selectedProduct.id,
name: name.trim(),
environment: environment.trim(),
url: url.trim(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should use zod here to verify data, as it stands everything can be inserted as an url, even javascript

if (!section || !parent || !child) return;

const compute = () => {
const sectionRect = section.getBoundingClientRect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this will break the animation if the user resize his window

}
}

$effect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i think this could be removed by directly calling initForm in handleDeleteCancel

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.

Dialog for adding external Services to the UI

3 participants