Skip to content

refactor: remove the visibility property from documents and pages#1511

Open
Kenny806 wants to merge 2 commits intomainfrom
refactor/visibility-cleanup
Open

refactor: remove the visibility property from documents and pages#1511
Kenny806 wants to merge 2 commits intomainfrom
refactor/visibility-cleanup

Conversation

@Kenny806
Copy link
Copy Markdown
Contributor

@Kenny806 Kenny806 commented Mar 27, 2026

Removing the visibility property from Document and DocumentPage. As the property is never actually returned from our APIs, it is always undefined and we do not have any use case for it at the moment. This also removes a lie from our type definitions :-)

@Kenny806 Kenny806 requested review from a team as code owners March 27, 2026 12:12
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 27, 2026

🦋 Changeset detected

Latest commit: 589034c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@frontify/guideline-blocks-settings Patch
@frontify/app-bridge Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Kenny806 Kenny806 requested review from SamuelAlev and removed request for syeo66 March 27, 2026 12:12
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Collaborator

@ragi96 ragi96 left a comment

Choose a reason for hiding this comment

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

This is a breaking change please do it on the dev branch (we then just need to think about how we wanna map that, for v3)

@Kenny806
Copy link
Copy Markdown
Contributor Author

Kenny806 commented Mar 27, 2026

This is a breaking change please do it on the dev branch (we then just need to think about how we wanna map that, for v3)

@ragi96 Is it? The value is never populated and is always undefined. If anyone was ever trying to read document.visibility, they always got undefined. And they will continue to get undefined even after this is merged. Or am I missing something here?

@ragi96
Copy link
Copy Markdown
Collaborator

ragi96 commented Mar 27, 2026

This is a breaking change please do it on the dev branch (we then just need to think about how we wanna map that, for v3)

@ragi96 Is it? The value is never populated and is always undefined. If anyone was ever trying to read document.visibility, they always got undefined. And they will continue to get undefined even after this is merged. Or am I missing something here?

Really? 😆
I need to check it out quickly :)

@ragi96
Copy link
Copy Markdown
Collaborator

ragi96 commented Apr 7, 2026

Finally had time to retest and imo it's still a breaking change:

Screenshot 2026-04-07 at 16 54 14

Feel free to remove it in v4 and map it to something static in the web-app, but we can't change this production type.

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