-
Notifications
You must be signed in to change notification settings - Fork 25
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
New Member Model #616
New Member Model #616
Conversation
// Member's participation status is updated once they lose a round (updated as soon as a new value is known), | ||
// ie. a member's opt-in participation status lifetime is only from the start of Round 1 | ||
// until the end of the Round they lose (or end of the election) | ||
participatingInNextElection: boolean; |
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.
Nit: Due to above comment maybe a better name could be participatingInElection -- a little ambiguous, but I think it fits better the description....
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.
Good call. Done.
It's looking great! I think as we talked a little before, having some levels of member data amount is better for having smaller queries and less data bloat, but since queries are running locally it seems like there's not much gain in splitting it for now!
It's not, we need to work on it.
I think we have a couple options:
Both suggestions would only apply to receiving new data. It does not help at all with querying local data (which I imagine does not happen to production anyway, since we are also querying local)
OOps... this sounds bad. Yeah, I think the fix must be done in the subchain library level, that way we leverage this in new apps and also to other devs too. |
I chatted with Todd about this. The difference is that the production version with dfuse only sends blocks to connected clients when a relevant block is produced. The ship implementation doesn't do that filtering yet. In that case, though, even the production version with dfuse will cause all components connected to any box query to rerender when an updated block comes in. So yeah, we'll definitely want to fix that in the subchain lib. |
Opened an issue for that in #617 |
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.
LGTM!
Background
We've confusingly had two separate models for member info:
EdenMember
containing info from our contract andMemberData
containing member profile info from AtomicHub NFTs. We often have to awkwardly combine this information in our UI.Now that we're moving the subchain and deemphasizing NFTs, we have an opportunity to simplify this.
In this PR
In this PR, I create a new model/interface:
Member
. And I apply that interface to:NavProfile
componentObviously, the old model has to co-exist until we can move our UI over to the new--and that will make it a bit messy until then.
Questions and observations
NavProfile
component (when depending on ephemeral chain) re-renders multiple times per second. This is not gonna be great for performance. My guess is that it's because the subchain library's returning a new object with every update. I could wrap the box query hooks in another hook that does a deep compare of the result and only returns a new object if there were actual changes, but that's probably better done in the subchain library itself. Thoughts?After this PR
I'll migrate other parts of the frontend codebase to the new
Member
model. Most immediately, this could be done for the Community page/Members List. That will involve updating member chips too...which are consumed by elections UI. And elections need a few more items added to subchain before I can fully move that over. I'll start navigating that tomorrow, and may recruit from @sparkplug0025 or @tbfleming getting the missing properties added.