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
Allow developers to extend StreamValue #11830
base: main
Are you sure you want to change the base?
Conversation
Manage this branch in SquashTest this branch here: https://ababiccustom-streamvalue-suppo-xetco.squash.io |
0cee20a
to
5b1c9d8
Compare
wagtail/fields.py
Outdated
@@ -111,6 +111,10 @@ def __init__(self, block_types, use_json_field=True, **kwargs): | |||
|
|||
self.stream_block.set_meta_options(block_opts) | |||
|
|||
@property | |||
def value_class(self): | |||
return self.stream_block.value_class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in __init__
we have:
if isinstance(block_types, Block):
# use the passed block as the top-level block
self.stream_block = block_types
but StreamField really expectes BaseStreamBlock
(which inherits from Block
). I wonder if the check should change to BaseStreamBlock
, or perhaps this to use getattr
.
I couldn't find any tests of what happens if you try to initialize StreamField with a non-StreamBlock block. Perhaps I am overthinking this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any tests of what happens if you try to initialize StreamField with a non-StreamBlock block. Perhaps I am overthinking this
This isn't currently supported (but might be in future, once #9746 lands).
77a115c
to
70cd03e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wagtail/blocks/stream_block.py
Outdated
def __init__(self, local_blocks=None, search_index=True, **kwargs): | ||
self._constructor_kwargs = kwargs | ||
self.search_index = search_index | ||
self.value_class = kwargs.get("value_class", self.default_value_class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should define this as part of the inner Meta
class, like BaseStructBlock does - that way, subclasses can define this in their own Meta class, which is probably a more natural way of working than passing it to the StructBlock constructor.
…class instead of just StreamBlock (it's a sensible default for both)
…ayed attribute assignment
70cd03e
to
b2c79f2
Compare
17d6a66
to
c75b7f9
Compare
2bf0bd0
to
24f63dd
Compare
24f63dd
to
020c3a5
Compare
I have encountered a couple of situations where I wanted to customise the behaviour of
StreamValue
:StreamField
that doesn't include block definitions in migrations, I wanted to override the logic inStreamValue.to_python()
that strips out values for blocks that are not recognised.StreamValue
to autogenerate unique IDs for heading blocks and use them (and the heading text) to generate a structured 'minimap' for users.However, using anything other than the built-in
StreamValue
class is currently quite difficult, due to various assumptions in the codebase. This PR aims to address these assumptions, allowing the value class to be more easily substituted.