Skip to content

feat: replaced pink 1 with pink2 in documents permission component #2128

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.

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Jul 17, 2025

What does this PR do?

Replaced pink 1 with pink 2 alert component n documents permission component

Test Plan

image image image

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

yes

@coolify-appwrite-org
Copy link

coolify-appwrite-org bot commented Jul 17, 2025

The preview deployment is ready. 🟢

Open Preview | Open Build Logs

Last updated at: 2025-07-21 12:56:36 CET

@coolify-appwrite-org
Copy link

coolify-appwrite-org bot commented Jul 17, 2025

The preview deployment is ready. 🟢

Open Preview | Open Build Logs

Last updated at: 2025-07-21 12:59:39 CET

@HarshMN2345 HarshMN2345 requested a review from ItzNotABug July 17, 2025 12:27
@HarshMN2345 HarshMN2345 self-assigned this Jul 17, 2025
Copy link
Member

@ItzNotABug ItzNotABug left a comment

Choose a reason for hiding this comment

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

Pretty sure we have multiple occurrences of the old design Alert. Lets replace all of those.

@HarshMN2345 HarshMN2345 requested a review from ItzNotABug July 18, 2025 06:23
{:else}
<Alert type={alertType} isStandalone>
<Alert.Inline status={alertType === 'default' ? 'info' : alertType}>
Copy link
Member

Choose a reason for hiding this comment

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

lets make the alertType the same type as status from Alert.Inline so we don't have to do these kind of checks.

@@ -343,7 +344,8 @@
{/each}
</Table.Root>
<div class="u-flex u-flex-vertical u-gap-16">
Copy link
Member

Choose a reason for hiding this comment

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

Lets use the Layout.Stack here.

source: 'function_keys_card',
destination: 'docs'
})}
<div class="u-margin-block-start-24">
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this margin here?

})}
<div class="u-margin-block-start-24">
<Alert.Inline status="warning" dismissible on:dismiss={() => (showAlert = false)}>
<svelte:fragment slot="title">Your function is outdated</svelte:fragment>
Copy link
Member

Choose a reason for hiding this comment

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

Use the title prop on Alert.Inline.

<svelte:fragment slot="title">Your function is outdated</svelte:fragment>
Update your function version to make use of new features including build commands and
HTTP data in your executions. To update, follow the steps outlined in our
<a
Copy link
Member

Choose a reason for hiding this comment

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

We could try using the Link/Anchor components here.

@@ -45,7 +46,7 @@
<svelte:fragment slot="aside">
{#if isCloud}
{@const size = humanFileSize(sizeToBytes(service, 'MB', 1000))}
<Alert type="info">
<Alert.Inline status="info">
<p class="text">
Copy link
Member

Choose a reason for hiding this comment

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

we don't need this class or the p tag itself. Pl. verify.

@HarshMN2345 HarshMN2345 requested a review from ItzNotABug July 21, 2025 12:47
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