Skip to content

feat(dos-donts): add assets file and tests - #1383

Open
benfrontify wants to merge 11 commits into
mainfrom
benlammlin/gcm-200-add-file-dosdontsassetstsx-dosdontsitemstsx
Open

feat(dos-donts): add assets file and tests#1383
benfrontify wants to merge 11 commits into
mainfrom
benlammlin/gcm-200-add-file-dosdontsassetstsx-dosdontsitemstsx

Conversation

@benfrontify

Copy link
Copy Markdown
Contributor

add assets file and tests

@benfrontify
benfrontify requested a review from a team as a code owner June 30, 2026 08:48
@benfrontify
benfrontify requested review from hochreutenerl and ragi96 and removed request for hochreutenerl June 30, 2026 08:48
@@ -1,7 +1,6 @@
/* (c) Copyright Frontify Ltd., all rights reserved. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The ref is already typed useRef<DosDontsAssetsRef>(null) and DosDontsAssetsRef.openUpload is non-optional, so this cast is redundant and actively harmful, it re-types the ref to a weaker shape where the method is optional. If openUpload is ever renamed/removed on the ref, TypeScript won't flag this call and the menu action silently no-ops at runtime. Call the typed ref directly.

Also for the other not only openUpload

@benfrontify
benfrontify requested a review from ragi96 July 6, 2026 12:53
Comment thread packages/dos-donts-block/src/DoDontItem.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.spec.tsx
@benfrontify
benfrontify requested a review from ragi96 July 7, 2026 12:36

@ragi96 ragi96 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This 2 are important ones, If you don't get why we need to surpress it here, please reach out to me

Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
@benfrontify benfrontify changed the title feat(dos-donts): add assets file and tests feat(dos-donts): add assets fil§§d tests Jul 14, 2026
@benfrontify benfrontify changed the title feat(dos-donts): add assets fil§§d tests feat(dos-donts): add assets file and tests Jul 14, 2026
@benfrontify
benfrontify requested a review from ragi96 July 14, 2026 05:32
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
@benfrontify
benfrontify requested a review from ragi96 July 14, 2026 09:42
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
@benfrontify
benfrontify force-pushed the benlammlin/gcm-200-add-file-dosdontsassetstsx-dosdontsitemstsx branch from a6be2f1 to dd55c24 Compare July 23, 2026 14:36
@benfrontify
benfrontify requested a review from ragi96 July 23, 2026 15:27
Comment thread packages/dos-donts-block/src/DoDontItem.tsx Outdated
Comment thread packages/dos-donts-block/src/DosDontsAssets.tsx Outdated
@benfrontify
benfrontify requested a review from fulopdaniel July 28, 2026 09:46
@benfrontify

Copy link
Copy Markdown
Contributor Author

I’m not 100% sure if I resolved the drilling issue. Let me know how this attempt looks.

@fulopdaniel

Copy link
Copy Markdown
Collaborator

I’m not 100% sure if I resolved the drilling issue. Let me know how this attempt looks.

@benfrontify can you kindly explain to me the drilling issue you are trying to solve and your attempt at solving it?

By drilling issue I suspect it's about avoiding to send down a bunch of props through several components?

@benfrontify

Copy link
Copy Markdown
Contributor Author

{...props} i used this and Raffi commented on this that I shouldn't do thrilling or pass them one by one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants