Skip to content

UI cleanup 5712 continued - #5798

Open
Tiggles wants to merge 40 commits into
masterfrom
ui-cleanup-5712-continued
Open

Tiggles wants to merge 40 commits into
masterfrom
ui-cleanup-5712-continued

Conversation

@Tiggles

@Tiggles Tiggles commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Fixes remaining under #5712

Tiggles added 30 commits August 25, 2026 10:34
Adds expiration and project role (both fe and be)
ApiTokenBrowse handled by new API
Move where selection button is added (for most cases)
Change name to ContainerSize, because that's what it is for
Fix logic that straight up didn't work for JobBrowse
Fix sensitivity overwriting button
Refactor row rendering code
Frontend for #5779
Restrict "All users"-actions
@Tiggles
Tiggles requested a review from DanThrane September 23, 2026 09:23

@DanThrane DanThrane 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.

Mostly a few concerns, can you look into these? It might be that none of these are actually a problem, but I suspect at least the useCloudAPI one might cause some problems.


const [state, dispatch] = useReducer(dataFetchReducer, {
loading: false,
loading: true,

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 think this needs to depend on if callParametersInitial if noop is true or not. If it is true then we end up in a forever loading state that might not be cleared. I am not sure if this breaks anywhere in our UI today, I think we normally just send off a refetch right away, but it might break something down the line. Should also be an issue fix.

window.setTimeout(() => {
dialogStore.addDialog(<>
{linkInfo.loading ? <Spinner /> :
linkInfo.error ? <div className={Container}>

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.

This will never be hit since data is still null (and thus was filtered out on line 35).

const useButton = browser.defaultButtonRenderer(opts.selection, ip);
if (useButton) row.stat3.append(useButton);
} else if (ip.status.boundTo.length === 1) {
browser.on("renderStat4", (ip, stat) => {

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 haven't run the code, but is this the correct one? Based on the columns from line 103, I would assume that this should be renderStat2?

[ContainerSize.LARGE]: renderLarge,
};

function statFromContainerSize(row: ResourceBrowserRow, containerSize: ContainerSize): HTMLElement {

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 am unsure about this code, I haven't read it too carefully, but I am surprised that this code isn't considering which columns are defined in the resource browser. I would have assumed that this depends on the column count.

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