Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(AssetCard): align possible entity status #817

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

Spring3
Copy link
Contributor

@Spring3 Spring3 commented Jan 27, 2021

Purpose of PR

Align possible entity statuses with field-editor-helpers

forma was missing the deleted status

PR Checklist

  • I have read the relevant readme.md file(s)
  • All commits follow our Git commit message convention
  • Tests are added/updated/not required
  • Tests are passing
  • Storybook stories are added/updated/not required
  • Usage notes are added/updated/not required
  • Has been tested based on Contentful's browser support
  • Doesn't contain any sensitive information

@netlify
Copy link

netlify bot commented Jan 27, 2021

Deploy preview for forma-36 ready!

Built with commit 5c0f0d1

https://deploy-preview-817--forma-36.netlify.app

Copy link
Collaborator

@denkristoffer denkristoffer left a comment

Choose a reason for hiding this comment

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

And just to be sure, there's not meant to be any visual changes for deleted assets?

@Spring3
Copy link
Contributor Author

Spring3 commented Jan 28, 2021

I am not aware of any required visual changes to the Asset card in a deleted state.

If that would be required, then we should be able to address it in the next PR, but here I just added the missing possible value as a status because using AssetCard in typescript environment doesn't allow the "deleted" as a value for status and then it requires to do extra work each time to convert type to match the one AssetCard expects.

I mean, the "deleted" status is not new and it has been used like this for long time even with deleted status, just by "hacking" typing. (Omit<"deleted", status>) So I think that it's safe to assume that the card in a "deleted" status should not look any different from any other status. Unless you or someone else have an idea and think that we should make it look any different (but then, probably in a separate PR)

@mshaaban0
Copy link
Collaborator

I think deleted and archived should have the same styles, similar to Tag component?
Also I'm not sure if there's a better way to centralise entity-types, maybe a separate package like browser config?
//cc @suevalov

@Spring3
Copy link
Contributor Author

Spring3 commented Jan 28, 2021

Or maybe using something like contentful-types where we expose these. Cause I think its too small of a change for another package. But these are just some of my thoughts off the top of my head

@denkristoffer denkristoffer merged commit 94ad576 into master Jan 29, 2021
@denkristoffer denkristoffer deleted the chore/align-entity-status branch January 29, 2021 10:31
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.

None yet

3 participants