Skip to content
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/proposal form loads if chain data not loaded yet #3990

Merged
merged 13 commits into from
Jun 9, 2023

Conversation

mhagel
Copy link
Contributor

@mhagel mhagel commented May 22, 2023

Link to Issue

Closes: #3988

Description of Changes

  • Inits chain if API not loaded yet on new proposal page
  • reusable useInitChain hook

Test Plan

  • ran proposals.spec.ts e2e tests
  • CA (click around) tested on local:
    • http://localhost:8080/evmos/new/proposal shows "Feature not supported yet for this community"
      • loads on direct url
      • loads from "Create onchain proposal" click
    • repeat for any other chain: loads Proposal form
    • Click around between multiple communities in sidebar
      • sometimes click to Proposals first
      • sometimes click to New Onchain Proposal first

Deployment Plan

Other Considerations

@mhagel mhagel changed the title Mark.3988 new prop /new/proposal form loads if gov module not loaded yet May 22, 2023
@mhagel mhagel marked this pull request as ready for review May 22, 2023 22:57
Copy link
Collaborator

@jnaviask jnaviask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to know some constraints around when this hook is safe to use. Will it cause race conditions if e.g. app.chain is undefined? We should also document on the hook itself how to determine when it's complete & update dependent components.

@mhagel mhagel changed the title /new/proposal form loads if gov module not loaded yet /new/proposal form loads if chain data not loaded yet May 25, 2023
@mhagel
Copy link
Contributor Author

mhagel commented May 25, 2023

Would love to know some constraints around when this hook is safe to use. Will it cause race conditions if e.g. app.chain is undefined? We should also document on the hook itself how to determine when it's complete & update dependent components.

@jnaviask
Added a new check for app.preloadingChain, and documented on the hook how it works.

Also added evmos "not supported" message per this discussion:
https://commonxyz.slack.com/archives/C051XFN57FT/p1684858677872149?thread_ts=1684779785.894519&cid=C051XFN57FT

@mhagel mhagel requested a review from jnaviask May 25, 2023 18:45
Copy link
Collaborator

@jnaviask jnaviask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it still triggers a double init despite the hasFetched ref.

@mhagel mhagel marked this pull request as draft May 31, 2023 20:47
mhagel added 2 commits May 31, 2023 15:14
disable form on evmos
add documentation to hook
check if chain.preloading
@mhagel
Copy link
Contributor Author

mhagel commented May 31, 2023

Looks like it still triggers a double init despite the hasFetched ref.

I removed the cleanup function. This was resetting the ref when I didn't expect it to. Now just running once.

Also removed the check for preloading. Not needed.

@mhagel mhagel marked this pull request as ready for review May 31, 2023 21:49
@mhagel mhagel requested a review from jnaviask May 31, 2023 21:49
@jnaviask
Copy link
Collaborator

jnaviask commented Jun 7, 2023

@mhagel Ian added a new chain init call in #4138 -- can you update this PR to include it, and then we can merge?

@jnaviask jnaviask merged commit a3a95f6 into master Jun 9, 2023
@jnaviask jnaviask deleted the mark.3988-new-prop branch June 9, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/new/proposal infinite bobber
2 participants