Eng 3496 improve ux saas validation#8166
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
5de2d04 to
5183212
Compare
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned Files
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (93.72%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8166 +/- ##
==========================================
+ Coverage 85.40% 85.47% +0.07%
==========================================
Files 649 651 +2
Lines 42283 42434 +151
Branches 4960 4982 +22
==========================================
+ Hits 36112 36272 +160
+ Misses 5063 5055 -8
+ Partials 1108 1107 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hold, Branch history got really polluted, fixing it before re-requesting review |
5183212 to
f32b826
Compare
jpople
left a comment
There was a problem hiding this comment.
Couple code nits, but overall looking good, I won't block merge on this.
| { ssr: false }, | ||
| ); | ||
|
|
||
| export const EditorWithCopy = ({ yaml }: { yaml: string }) => ( |
There was a problem hiding this comment.
Feels a bit weird to be making this its own component, do we feel like this is likely to get used elsewhere or is it just a one-off? If the latter I might prefer just inlining the copy button.
There was a problem hiding this comment.
Its a One off for now, so it makes sense to just inline it, will do now
| return <YamlBlock yaml={configYaml} />; | ||
| if (!configYaml) { | ||
| return ( | ||
| <Text type="secondary" className="text-sm"> |
There was a problem hiding this comment.
| <Text type="secondary" className="text-sm"> | |
| <Text type="secondary" > |
Generally prefer not using Tailwind for text size, Ant doesn't offer an API to directly change size but I think it's fine to have this at the default size.
Ticket ENG-3496
Description Of Changes
Improving view of SaaShistory using the improved YAML editor available at fides
Adding aswell a copy to clipboard functionality for ease of debug and sharing information with technical staff
Code Changes
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works