feat: introduce link custom renderer #292
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This is being used in components like Button, InfoCols, etc. The follow-up is just adding the full functionality to support adding internal links, external links, selecting files and adding validation.
Full validation not available yet, but it would not just be the URL format. We have internal links that we store as references rather than as URLs. |
dcshzj
force-pushed
the
feat/link-custom-renderer
branch
from
0c0bd6f
to
103a98a
Compare
| <FormControl isRequired={required}> | ||
| <FormLabel description={description}>{label}</FormLabel> | ||
|
|
||
| <Tabs variant="group" onChange={() => handleChange(path, "")}> |
There was a problem hiding this comment.
Not really an issue, but does this mean that if I had data in a tab, and I clicked on another type of link(maybe to check if this suits the purpose better), and I clicked back my previous data will be lost?
There was a problem hiding this comment.
Yeah, the 4 types are mutually exclusive so the content of one type would not be transferrable to another, hence opted to clear the value to force the user to make the selection first.
...tures/editing-experience/components/form-builder/renderers/controls/JsonFormsLinkControl.tsx
Outdated
Show resolved
Hide resolved
...tures/editing-experience/components/form-builder/renderers/controls/JsonFormsLinkControl.tsx
Outdated
Show resolved
Hide resolved
| <Input | ||
| type="text" | ||
| value={dataString} | ||
| onChange={(e) => handleChange(path, e.target.value)} | ||
| placeholder="Page permalink" | ||
| /> |
There was a problem hiding this comment.
this however is the bigger issue (and thus i'd recommend dynamic state based on selected radio value instead), as the tab panels can desynchronise with LINK_TYPES and a specific order is required, which is not obvious if LINK_TYPE is ever modified.
might be safer to not use tabs and just render a specific input depending on the selected radio option. to that end, you can rely on compile time typechecking to ensure something is rendered accordingly to the option.
UNLESS the value of the current tab does not matter to you, and only the input value does. then maybe tab can be used.
There was a problem hiding this comment.
Updated in 950c8ed, agreed that it would better with a more explicit mapping of the link type to the actual rendering.
The value of the current tab would not matter because the 4 link types are mutually exclusive, so it is not possible to translate from one to another. Opted to clear out the value of the input to avoid confusing the user.
dcshzj
force-pushed
the
feat/link-custom-renderer
branch
from
cf92a66
to
4198ea8
Compare
| /> | ||
| ) | ||
| default: | ||
| return <></> |
There was a problem hiding this comment.
nit: might want to skip default case, or assert never for compile-time typecheck
see knowledge base
...tures/editing-experience/components/form-builder/renderers/controls/JsonFormsLinkControl.tsx
Outdated
Show resolved
Hide resolved
...tures/editing-experience/components/form-builder/renderers/controls/JsonFormsLinkControl.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const handleLinkTypeChange = (value: string) => { | ||
| setSelectedLinkType(value) | ||
| handleChange(path, "") |
There was a problem hiding this comment.
does this actually do anything? are you supposed to pass in the change into handleChange too?
There was a problem hiding this comment.
Hmm yeah it is supposed to reset the input field to empty when they change the selected link type. This is because all 4 link types are mutually exclusive of one another, so the input from one does not translate to another, so opted to clear the input field to prevent user confusion.
dcshzj
force-pushed
the
feat/link-custom-renderer
branch
from
5e33dc3
to
313431b
Compare
Problem
We need a custom renderer to insert links.
Solution
Breaking Changes
Features:
Before & After Screenshots
BEFORE:
AFTER: