Removed contractId and added made contracts required, signAndSendTran…

[amirsaran3]

Description

This PR is an extension of #811. Addressing comments of Daryl on mentioned PR.

Breaking changes:

• Removed contractId from setupModal params
• contracts param in setupModal is now required
• receiverId param in signAndSendTransaction is now required since we can't assume a default contract since we can have multiple

TODO:

• Since none of the wallets support the signInMulti method right now we can't test it correctly. I am not sure how will the pending state act after these changes (for browser wallets).

Questions:

• signInMulti method with one contract in contracts param list is same as calling signIn method? Any notable differences?
• Regarding this comment: feat: Add signInMulti method #811 (comment). If user first signs in using two contracts and then one of them expires. He then only wants to re-sign in to the expired one and the other one is still signed in. How will the UI flow work for this interaction? Since the user is signed in they will not see a prompt to sign in again.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings

Type of change.

• FIX - a PR of this type patches a bug.
• FEATURE - a PR of this type introduces a new feature.
• BUILD - a PR of this type introduces build changes.
• CI - a PR of this type introduces CI changes.
• DOCS - a PR of this type introduces DOCS improvement.
• STYLE - a PR of this type introduces style changes.
• REFACTOR - a PR of this type introduces refactoring.
• PERFORMANCE - a PR of this type introduces performance changes.
• TEST - a PR of this type adds more tests.
• CHORE - a PR introduces other changes than the specified above.

[AZbang]

@amirsaran3 when it will be merged?

[dinhthuybg]

git stash

[gtsonevv]

@amirsaran3 @kujtimprenkuSQA Can we merge this PR?

[kujtimprenkuSQA]

Hi @gtsonevv, my comments are mostly around the TS type for the contracts in the core package I left a detailed comment to explain the reason for this suggestion in the store.ts file.

If you don't think this is a necessary change you might ignore them 👍 , but if you do the change then you need to update the checks in signAndSendTransaction(s)

// From this
// Note: this is just an example.
if (!_state.wallet.isSignedIn() || contracts.length < 1) {
  throw new Error("Wallet not signed in");
}
      
// To something like this:
if (!_state.wallet.isSignedIn() || !contracts) {
  throw new Error("Wallet not signed in");
}

The docs / readme.md in modal-ui and modal-ui-js must be updated to reflect the changes of this MR.

[kujtimprenkuSQA]

@kujtimprenkuSQA Can we merge this PR?

@gtsonevv the PR looks good to me, I think the docs should be updated because this PR introduces breaking changes so the docs will be outdated.

I only reviewed the code which looks good to me, but the decision to merge it needs to be made by your team.

You can mark my previous comments as resolved 👍

[gtsonevv]

@kujtimprenkuSQA Perfect! Which docs exactly?

[kujtimprenkuSQA]

@kujtimprenkuSQA Perfect! Which docs exactly?

The code example and the Options in this file (there's no contract anymore only contracts) refer to the code changes for better understanding:
https://github.com/near/wallet-selector/tree/SQC-540/add-signin-multi-breaking-change/packages/modal-ui

The code example and the Options on this file:
https://github.com/near/wallet-selector/tree/SQC-540/add-signin-multi-breaking-change/packages/modal-ui-js

This PR removes the .contract from the selector state:
https://github.com/near/wallet-selector/blob/SQC-540/add-signin-multi-breaking-change/packages/core/docs/api/state.md#contract

In signAndSendTransaction the receiverId is now required (remove the ?):
https://github.com/near/wallet-selector/blob/SQC-540/add-signin-multi-breaking-change/packages/core/docs/api/wallet.md#signandsendtransactionparams

[pivanov]

r+