Skip to content

Extend properties CDDL with attributes #4243

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

casey
Copy link
Collaborator

@casey casey commented Feb 26, 2025

Fixes #4222. As a first stab, just extend the CDDL.

To do:

  • write tests for all different value types
  • make sure trait order is preserved
  • how should gallery item metadata be displayed?
  • hidden metadata
  • look at existing collection attributes
  • add a value for "none", since this seems to be common

@casey casey mentioned this pull request Feb 26, 2025
@lifofifoX
Copy link
Collaborator

Few more things came to mind.

  • alt text is going to be same as title in majority, if not all, of the cases. Why not start without it?
  • Most links will likely require some label. How about including labels in the ? 4: [+ uri], ; links attribute? I understand people can just use a trait instead. But seems odd to have both. Maybe ? 4: [+ uri], ; links can be removed altogether in favor of traits.
  • Traits value type list seems extensive. Why not start small and expand as we go along, in order to avoid having types almost no one ever uses?

@casey
Copy link
Collaborator Author

casey commented Mar 8, 2025

  • alt text is going to be same as title in majority, if not all, of the cases. Why not start without it?

alt text is a description, not a title. Although I think maybe it's mostly superfluous with title and description, even though alt text is supposed to be a literal description of the content, so I removed it.

  • Most links will likely require some label. How about including labels in the ? 4: [+ uri], ; links attribute?

I'm wondering about this. I think most links are explanatory. For example, if it's a link to discord.gg, we know it's Discord, and can even give it a Discord logo.

I understand people can just use a trait instead. But seems odd to have both. Maybe ? 4: [+ uri], ; links can be removed altogether in favor of traits.

I think that they're presented differently. Traits should be grouped under together, but project links definitely don't belong with traits.

This seems better:

links:
- https://discord.gg
traits:
- hat: red
- background: black

Than this:

traits:
- discord: https://discord.gg
- hat: red
- background: black
  • Traits value type list seems extensive. Why not start small and expand as we go along, in order to avoid having types almost no one ever uses?

The all seem reasonably useful and reasonably easy to implement. But if you think one in particular isn't useful, then feel free to suggest that we remove it.

@lifofifoX
Copy link
Collaborator

But if you think one in particular isn't useful, then feel free to suggest that we remove it.

Proportion and bytes stood out as something that might not get much use.

tdate and time seem to achieve the same. Is there a scenario where you'd want to use one over the other?

@casey
Copy link
Collaborator Author

casey commented Mar 8, 2025

Proportion and bytes stood out as something that might not get much use.

Proportion is actually one of the very few data types which is supported by OpenSea, so I think we should have it for coverage of what people use for ERC-721s. I think it also gets some use for "power level" type stuff.

Bytes I agree is maybe niche, but raw bytes are elemental, so I definitely want people to be able to use them.

tdate and time seem to achieve the same. Is there a scenario where you'd want to use one over the other?

tdate has a time zone, so it's a localized time. time is a time-zone-free integer timestamp. I definitely see how they overlap. Although I think they're both useful.

My thinking is that I definitely usually think changes should be minimal, but adding different value types just seems easy enough.

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.

Attributes
2 participants