Skip to content

feat: parse resource metadata values as markdown #10521

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

Merged
merged 11 commits into from
Nov 7, 2023
Merged

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Nov 3, 2023

Closes coder/terraform-provider-coder#115

Screenshot 2023-11-06 at 12 28 11 PM
  • Parse resource metadata values as a limited subset of markdown, that will reduce the ability to render values which misbehave/break out of boxes/cause weird layout issues

  • Only enable "click to copy" on simple, unformatted text. This seemed to be a nice compromise between complexity and functionality.

    • It avoids requiring the user to configure whether they want something to be copyable, or if they want it to render as markdown
    • It avoids things like having a clickable link inside of the CopyableValue which looks weird and feels bad, and deciding what value should be copied if it has bold text, etc.

@aslilac aslilac requested review from a team and Parkreiner and removed request for a team November 6, 2023 19:34
Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

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

Everything seems to make sense to me. Just had some clarifying questions about how things are set up, and flagged a bug with the memoization

@aslilac aslilac requested a review from Parkreiner November 6, 2023 20:48
Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

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

I'm not going to block on this, but I still feel like it might make sense to give memo a custom comparison function

My worry is that with how subtle the memoization bug was here, anyone using the component in the future might accidentally have the exact same problem (and worse yet, might not realize that they're doing anything wrong)

@aslilac
Copy link
Member Author

aslilac commented Nov 7, 2023

yeah yeah, I can use _.isEqual :^)

@aslilac aslilac merged commit 9e4558a into main Nov 7, 2023
@aslilac aslilac deleted the markdown-resource-value branch November 7, 2023 17:34
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Parse coder_metadata resource item as markdown
2 participants