#5921 - user feedback: add documentation link in page editor #5982
Conversation
…5921-user-feedback-add-documentation-link-in-page-editor
grahamlangford
requested review from
twschiller and
turbochef
and removed request for
twschiller and
turbochef
| @@ -56,13 +62,26 @@ const EditorNodeConfigPanel: React.FC = () => { | |||
| /> | |||
| ); | |||
|
|
|||
| const showDocumentationLink = !isLoadingListing && listings[blockId]?.id; | |||
There was a problem hiding this comment.
I think we only want to show the link if the "how to use" section is not empty for the listing? IIRC that was the old behavior we had
There was a problem hiding this comment.
There is a now-unused component for this called ConfigurationTitle. I think this PR should either:
- Rename/refactor/use that component for this docs link
OR - Delete that component if no longer needed
TBH, I originally thought we were just going to add the ConfigurationTitle component back in, in order to complete both this and the brick name task 🤷
Didn't know about this component, looking into it now. |
…urationTitle component
There was a problem hiding this comment.
What you have here is probably fine, and this is a minor feature, but it's not really handling async state in our newer idiomatic way using AsyncStateGate or other utils.
See here for an example of using AsyncStateGate to guard the async state instead of checking loading flags/values.
| const showDocumentationLink = | ||
| !isLoadingListing && listingInstructions && listingId; |
There was a problem hiding this comment.
Probably don't need to check the loading flag if you are also checking falsy values?
There was a problem hiding this comment.
We really don't. I just didn't think to remove it when I changed the other checks.
| @@ -30,6 +30,9 @@ import PopoverInfoLabel from "@/components/form/popoverInfoLabel/PopoverInfoLabe | |||
| import AnalysisResult from "@/pageEditor/tabs/editTab/AnalysisResult"; | |||
| import { useSelector } from "react-redux"; | |||
| import { selectActiveNodeInfo } from "@/pageEditor/slices/editorSelectors"; | |||
| import { MARKETPLACE_URL } from "@/utils/strings"; | |||
| import { useGetMarketplaceListingsQuery } from "@/services/api"; | |||
| import classNames from "classnames"; | |||
There was a problem hiding this comment.
The convention is to import as cx, import cx from "classnames". This would be a good lint rule too
There was a problem hiding this comment.
We have a linting epic for those now. I'll add that as a recommendation.
Codecov Report
@@ Coverage Diff @@
## main #5982 +/- ##
==========================================
+ Coverage 66.14% 66.23% +0.08%
==========================================
Files 1092 1091 -1
Lines 33716 33682 -34
Branches 6354 6342 -12
==========================================
+ Hits 22301 22308 +7
+ Misses 11415 11374 -41
|
…5921-user-feedback-add-documentation-link-in-page-editor
grahamlangford
deleted the
5921-user-feedback-add-documentation-link-in-page-editor
branch
What does this PR do?
Demo
Checklist