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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
18640 Incremental amalgamating businesses validations, etc #590
18640 Incremental amalgamating businesses validations, etc #590
Changes from 2 commits
059b635
a77986c
b55a7ce
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
The other way we could have done this is with an action that pushes the new business into the array right in the store, but I think this is OK.
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.
Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce
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.
Love 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.
One architecture/design thing I wanted to mention:
This is a computed value (local getter), which is reactive to the variables used in it, in particular
getAmalgamatingBusinesses
, so it is evaluated (and cached) accordingly.Also, as a computed, it needs to be used somewhere (otherwise it is not evaluated). In this component, it's used both to display the table items (line 17) and is watched to emit whether the table is valid (line 166).
Another way to do this would be to simply watch
getAmalgamatingBusinesses
and then compute the statuses when it has changed. If the getter approach didn't work (eg, reactivity didn't work) then I would have changed to this approach, otherwise I see no reason to change working code :)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.
WIP but some functionality is in and usable. I will pick away at this but I am no longer blocking anyone.
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.
WIP.
BTW: https://docs.google.com/document/d/1Dt2jzPA2SLtQO7Ns2aPCzVchGdv4YrYB/edit
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've requested access to the document.
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.
More WIP. Just assume everything in here is WIP, except for the basic architecture and design :)