Add slot_index method #986
Open
+46
−0
Conversation
|
@dkniffin it looks like you're making changes in the right places! FWIW, I generally prefer to see failing tests before anything else, so we can chat about proposed APIs. |
|
@joelhawksley Alright, I pushed up a commit w/ a failing test (and rebased so it's in the right order). I'm noticing now that the implementation is definitely a bit wrong because there's some test failures. Thoughts? |
| @@ -250,6 +256,10 @@ def set_slot(slot_name, *args, **kwargs, &block) | |||
|
|
|||
| if slot_definition[:collection] | |||
| @__vc_set_slots[slot_name] ||= [] | |||
|
|
|||
| # set the index to the next index in the array (aka, the current length) | |||
| slot.slot_index = @__vc_set_slots[slot_name].length | |||
BlakeWilliams
Jul 12, 2021
Member
What happens if we sort the collection before rendering?
What happens if we sort the collection before rendering?
dkniffin
Jul 13, 2021
Author
Contributor
:/ Yeah, that wouldn't work with the current implementation. I'm not sure if that'd be a common use-case or not though. It's certainly not for me.
:/ Yeah, that wouldn't work with the current implementation. I'm not sure if that'd be a common use-case or not though. It's certainly not for me.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Resolves #982
I've taken a first stab at this. @joelhawksley let me know what you think of this. I'd definitely want to add tests and docs, but I want to make sure I'm headed in the right direction first.