status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Move request to join UI state out of community object

Open ajayesivan opened this issue 1 year ago • 2 comments

Keeping the community request-to-join related data in the app-db under the community object is unnecessary and will cause unintended re-renders, as we are subscribing to the community object in various places. Our goal is to move the UI state of the request-to-join flow to a separate root-level objects indexed by community ID, so the UI changes won't affect the community object.

Data we need to move out of the community object

  • airdrop-address
  • previous-permission-addresses
  • selected-permission-addresses
  • previous-share-all-addresses?
  • share-all-addresses?

Move the data stored in the above keys to :communities/<key> { community-id {}}

ajayesivan avatar Feb 15 '24 15:02 ajayesivan

Hi @ajayesivan, Thank you very much for creating this issue and your work in https://github.com/status-im/status-mobile/pull/18843

I am getting all confused with these previous, and next states. This is a new territory where we need to store a temporary state and only persist once the confirm button is pressed. We don't have many examples across the app of this kind of behavior. So I think we need to come up with a coding guideline for this.

Currently what we are doing is while opening the related screens we are backing up the old state in the :previous-... key and if we press cancel then we are using that backup to revert (please correct if I am wrong). This doesn't look good to me, because,

  • we should not modify the original state until confirm is pressed.
  • it will cause rerender into screens that are subscribing to the original state
  • And if we somehow miss canceling then we arrive at a weird state - as mentioned in https://github.com/status-im/status-mobile/issues/18835

What I propose is,

  • While opening a related state create a new duplicate entry :local-....
  • Make changes to this state with actions
  • And replace the original state with this one on the confirm button.

cc @ilmotta @cammellos, please let me know if this sounds ok.

Parveshdhull avatar Feb 19 '24 09:02 Parveshdhull

Hi @Parveshdhull, Thank you for your insight. You're absolutely correct; the current approach of using the previous state and relying on it to reset the selected state is highly error-prone. In this refactoring, I plan to introduce an editing state as you mentioned instead of directly modifying the original state. Updates to the original state will only occur when the user confirms.

Please feel free to share any further insights you may have on this matter. When I create a PR, I'll be sure to request your review.

ajayesivan avatar Feb 19 '24 09:02 ajayesivan