Skip to content

permission request button for led sign page#1990

Merged
evanugarte merged 5 commits into
devfrom
Request-Button4LEDSIGN
Mar 9, 2026
Merged

permission request button for led sign page#1990
evanugarte merged 5 commits into
devfrom
Request-Button4LEDSIGN

Conversation

@wayngo

@wayngo wayngo commented Jan 3, 2026

Copy link
Copy Markdown
Collaborator

step 2

@adarshm11

Copy link
Copy Markdown
Contributor

you spelled permission wrong

@wayngo wayngo changed the title permsision request button for led sign page permission request button for led sign page Jan 9, 2026
@wayngo

wayngo commented Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author
image

Comment thread src/Pages/LedSign/LedSign.js Outdated
}, [])

if (loading) {
if (loading || checkingPermission) {

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 we can skip the second checkingPermission check, so the page loads as soon as the healthcheck is done

renderPermissionRequestUI can have the checkingPermission check instead, and it can have some small message like "loading", and then fade in the result (requested already or not yet)

Comment thread src/APIFunctions/PermissionRequest.js
Comment thread src/APIFunctions/PermissionRequest.js Outdated
Comment on lines +46 to +56
if (res.ok) {
// Backend sends 200 with no body on success, so fetch the created request
const existingRequest = await getPermissionRequest(type, token);
status.responseData = existingRequest.responseData;
} else if (res.status === 409) {
// If conflict, fetch the existing request
const existingRequest = await getPermissionRequest(type, token);
status.responseData = existingRequest.responseData;
} else {
status.error = 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.

would this work

Suggested change
if (res.ok) {
// Backend sends 200 with no body on success, so fetch the created request
const existingRequest = await getPermissionRequest(type, token);
status.responseData = existingRequest.responseData;
} else if (res.status === 409) {
// If conflict, fetch the existing request
const existingRequest = await getPermissionRequest(type, token);
status.responseData = existingRequest.responseData;
} else {
status.error = true;
}
status.error = !!res.ok;
if (res.ok || res.status === 409) {
// Backend sends 200 with no body on success, so fetch the created request
const existingRequest = await getPermissionRequest(type, token);
status.responseData = existingRequest.responseData;
}

Comment thread src/Pages/LedSign/LedSign.js Outdated
Comment on lines +299 to +301
<p className="text-sm text-gray-600 dark:text-gray-400 italic">
Drop a message in Discord to speed up the process
</p>

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.

Suggested change
<p className="text-sm text-gray-600 dark:text-gray-400 italic">
Drop a message in Discord to speed up the process
</p>

Base automatically changed from LED4MEMBERS to dev March 3, 2026 05:17
@evanugarte
evanugarte force-pushed the Request-Button4LEDSIGN branch from aa690ca to cfce7f9 Compare March 4, 2026 05:09
@evanugarte

Copy link
Copy Markdown
Collaborator

#1988 will handle this

@evanugarte evanugarte closed this Mar 8, 2026
@evanugarte evanugarte reopened this Mar 8, 2026
@evanugarte
evanugarte force-pushed the Request-Button4LEDSIGN branch from 39cc753 to 48db042 Compare March 9, 2026 00:33

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

lean 😹

@evanugarte
evanugarte merged commit f979019 into dev Mar 9, 2026
4 checks passed
@evanugarte
evanugarte deleted the Request-Button4LEDSIGN branch March 9, 2026 03:08
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.

4 participants