Link comments to scenes based on the commentId prop
#4795
Conversation
| @@ -11,10 +11,10 @@ import * as PP from '../shared/property-path' | |||
| import { assertNever } from '../shared/utils' | |||
| import { isSceneAgainstImports, isRemixSceneAgainstImports } from './project-file-utils' | |||
|
|
|||
| const IdPropName = 'id' | |||
| export const SceneCommentIdPropName = 'data-comment-id' | |||
There was a problem hiding this comment.
Since it's ONLY for scenes, could we make this just commentId? Scene is our component, so we we can make it a bit less clunky
There was a problem hiding this comment.
sure, I updated it in 7ebe371.
The reason I went with data-comment-id is because it fit the mold of the other Utopia-specific props on Scene (data-label, data-uid), and using data-comment-id didn't make it necessary to tweak the code of Scene (without the tweaking, a React error is triggered: React does not recognize the 'commentId' prop on a DOM element, since all props on Scene are forwarded to the underlying div).
Job #10095: Bundle Size — 59.83MiB (+0.01%).
Warning Bundle contains 66 duplicate packages – View duplicate packages Bundle metrics
 
Bundle size by type
View job #10095 report View feature/scene-comment-id-rename branch activity View project dashboard |
bkrmendy
changed the title
commentId prop
| @@ -11,10 +11,10 @@ import * as PP from '../shared/property-path' | |||
| import { assertNever } from '../shared/utils' | |||
| import { isSceneAgainstImports, isRemixSceneAgainstImports } from './project-file-utils' | |||
|
|
|||
| const IdPropName = 'id' | |||
| export const SceneCommentIdPropName = 'commentId' | |||
There was a problem hiding this comment.
small suggestion - maybe comment-target-id?
(since it's not the id of a comment, but the id of the scene as a comment target)
| @@ -81,7 +81,11 @@ const BasicUtopiaSceneDescriptor = ( | |||
| jsxElementWithoutUID( | |||
| name, | |||
| [ | |||
| jsxAttributesEntry('id', jsExpressionValue('scene', emptyComments), emptyComments), | |||
| jsxAttributesEntry( | |||
| 'commentId', | |||
There was a problem hiding this comment.
there isn't a way to use SceneCommentIdPropName here, right?
| @@ -7831,7 +7831,7 @@ packages: | |||
| dev: true | |||
|
|
|||
| /component-indexof/0.0.3: | |||
| resolution: {integrity: sha1-EdCRMSI5648yyPJa6csAL/6NPCQ=} | |||
| resolution: {integrity: sha512-puDQKvx/64HZXb4hBwIcvQLaLgux8o1CbWl39s41hrIIZDl1lJiD5jc22gj3RBeGK0ovxALDYpIbyjqDUUl0rw==} | |||
…to feature/scene-comment-id-rename
…to feature/scene-comment-id-rename
Problem
The relationship between the
idprop on Scenes and the position of Scene comments is not evident. If a user deletes theidprop from a scene, any comments attached to that scene would be reparented to the canvas, without any indication from us.Fix
Link the comments on scenes based on the value of the
commentIdprop, which better communicates the purpose of the prop