Skip to content

chore: add soft token + add subtle button variant#36899

Merged
jsartisan merged 2 commits intoreleasefrom
chore/chat-ui-fixes
Oct 16, 2024
Merged

chore: add soft token + add subtle button variant#36899
jsartisan merged 2 commits intoreleasefrom
chore/chat-ui-fixes

Conversation

@jsartisan
Copy link
Contributor

@jsartisan jsartisan commented Oct 16, 2024

/ok-to-test tags="@tag.Anvil"

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11362114942
Commit: e1fc66e
Cypress dashboard.
Tags: @tag.Anvil
Spec:


Wed, 16 Oct 2024 09:07:10 UTC

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Oct 16, 2024
@jsartisan
Copy link
Contributor Author

Note: This PR adds a new subtle variant that we will use for suggested prompt buttons in AIChat component.
In the next PR in ee, I'll replace the AssistantSuggestionButton with our button with subtle variant.

import type { ReactNode } from "react";
import type { COLORS } from "../../../shared";

export interface TextProps extends React.HTMLAttributes<HTMLElement> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why this was removed? This change will not allow to pass standard html attributes to Text

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what was happening was initially when creating markdown components for paragraphs, I was using ...props and passing it to our Text which resulted in type conflicts as props of paragraph were HTMLParagraphElement and our Text couldn't accept those props. So I added the above code to fix it. But later I removed ...props part. So not required anymore.

Having the above causes issues in storybook. It shows all props of HTMLelement in the docs. 🫠

CleanShot 2024-10-16 at 14 11 00

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks. I still believe that ability to pass standard html attributes to any components is required (do you agree?). I'm sure that mentioned issues can be fixed, but don't think we have time for it. So it is up to you, feel free to resolve the thread 👍

@jsartisan jsartisan added the ok-to-test Required label for CI label Oct 16, 2024
@jsartisan jsartisan merged commit 24d989d into release Oct 16, 2024
@jsartisan jsartisan deleted the chore/chat-ui-fixes branch October 16, 2024 09:44
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Nov 20, 2024
/ok-to-test tags="@tag.Anvil"

<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11362114942>
> Commit: e1fc66e
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11362114942&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Anvil`
> Spec:
> <hr>Wed, 16 Oct 2024 09:07:10 UTC
<!-- end of auto-generated comment: Cypress test results  -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants