"Main" branch clean-up #12

kpyszkowski · 2023-11-30T15:10:33Z

Ref: #1
Depends on: #7 #8 #10 #11

This pull request aggregates each partial clean-up PR (mentioned above). It's introduced to reduce complexity of merge conflicts and allow continuous work without blocking while waiting for partial PRs to be merged.
Moreover it's a place for general cleanup commits that couldn't be addressed to any of the PRs.

kpyszkowski · 2023-12-01T11:35:18Z

@michalsmiarowski
I've spotted an error that doesn't occur on branches #7, #8, #10, #11 but in this one. I might have messed something with resolving conflicts but I can't find the direct origin of the issue.
When you disconnect the wallet using a button (calling disconnect function returned by useWeb3React hook) after few seconds it throws an error from the screenshot. in fact it breaks in this line because as you disconnect the wallet, the account value from useWeb3React hook becomes undefined.
I'm curious how this error didn't occur earlier because from the code perspective that is "expectable" behavior.

Could you investigate it? I’ve already hit the wall, all my efforts in this matter are pointless 😩

Edit:
Okay, finally I have found a hint. Actually this happens when you disconnect wallet on /tBTC/(un)mint route.
That's because it tries to render the ActivityItem component that's incorrectly assumes the account value will always be present. When you disconnect wallet on any other route - the route that doesn't render the mentioned component - the error doesn't occur.

I've prepared a patch with testing playground in case you'd like to test it:

git apply ~/Downloads/route-test.patch

Edit 2:
Ahhh, finally I've managed to resolve the issue. The rubber duck method FTW 🤣
Ref commit: 5bf5c773

Component tried to render without the necessary data causing error when user disconnects the wallet on Mint/Unmint page.

michalsmiarowski

First round of review

src/App.tsx

michalsmiarowski · 2024-01-07T20:43:37Z

src/store/index.ts


 const combinedReducer = combineReducers({
  account: accountSlice.reducer,
  modal: modalSlice.reducer,
  token: tokenSlice.reducer,
  sidebar: sidebarSlice.reducer,
  transaction: transactionSlice.reducer,
-  staking: stakingSlice.reducer,
  eth: ethSlice.reducer,
  tbtc: tbtcSlice.reducer,
  rewards: rewardsSlice.reducer,


Non-blocking:

rewards are not needed anymore, because they are related to staking, so let's remove that slice and all things related to that.

I've made it non-blocking, because I think I can address it in a separate PR too to not distract you from the UI work and to not block this PR.

michalsmiarowski · 2024-01-07T20:45:23Z

src/store/index.ts


 const combinedReducer = combineReducers({
  account: accountSlice.reducer,
  modal: modalSlice.reducer,
  token: tokenSlice.reducer,
  sidebar: sidebarSlice.reducer,
  transaction: transactionSlice.reducer,
-  staking: stakingSlice.reducer,
  eth: ethSlice.reducer,


Non-blocking:

I'm pretty sure eth is also not needed here since we operate only on bitcoins in base, so we should remove that.

I've made it non-blocking, because I think I can address it in a separate PR too to not distract you from the UI work and to not block this PR.

michalsmiarowski · 2024-01-07T20:46:46Z

src/store/index.ts

 import { listenerMiddleware } from "./listener"
-import { accountSlice, registerAccountListeners } from "./account"
+import { accountSlice } from "./account"

 const combinedReducer = combineReducers({
  account: accountSlice.reducer,
  modal: modalSlice.reducer,
  token: tokenSlice.reducer,


Non-blocking:

token is needed, but we only need tbtc-v2 and all things realted to that. The rest can be removed.

Let me address it in a separate PR where I will remove useFetchTvl hook. We shouldn't add more changes here.

Fetching ETH USD price is not needed in Base app

michalsmiarowski

Left some comments to look at before the merge

src/components/tBTC/BridgeActivity.tsx

Updated conditions to include wallet connection. Fixed typo: `epmty` => `empty`

Adjust empty state rendering conditions.

michalsmiarowski

Left one more comment

src/components/tBTC/BridgeActivity.tsx

Used dedicated boolean flag instead of the data itself

michalsmiarowski

LGTM 🔥

UI: Header component Ref: #2 ~Depends on: #12~ This pull request introduces: - `Logo` component: - `Header` component - `SelectWallet` component redesign Additional changes: - `Button` component theme adjustments - `Modal` component theme adjustments Note: Since this PR introduces changes widely used in the project eg. `Modal` or `Button` changes it's recommended to review it and merge primarily.

UI: PageLayout component Ref: #2 ~Depends on: #12 This pull request introduces `PageLayout` component. I decided to rewrite the component entirely. The major feature is render slots to conditionally render content in specific areas regarding to designs. Render slots are wrapped with padding container following design appearance, children is not wrapped with any container (padding) since it's appears to be variable in various views. https://github.com/threshold-network/bitcoin-on-base/assets/11503879/deccabfb-3f6e-4f2f-a07a-76a162635e1b Note: The TODO regarding Balance component (#17) isn't a blocker. It's just a placeholder value indicating to put the component there when the view is composed in separate PR.

UI: Provide Data step Ref: #2 Depends on: #12 This pull request introduces redesigned Provide Data minting flow step

UI: Initiate Minting step Ref: #5 Depends on: #12 This pull request introduces redesigned Initiate Minting minting flow step. Additional changes: - `Toast` component with collapsible details, adjustable position and orientation, - `LabeledBadge` component.

kpyszkowski added 3 commits November 30, 2023 15:54

Merge branch 'staking-removal' into main-cleanup

b1651f6

Merge branch 'feedback-removal' into main-cleanup

5803b84

Merge branch 'overview-removal' into main-cleanup

e7d7f64

kpyszkowski self-assigned this Nov 30, 2023

kpyszkowski marked this pull request as draft December 1, 2023 10:42

Resolve ActivityItem component issue

5bf5c77

Component tried to render without the necessary data causing error when user disconnects the wallet on Mint/Unmint page.

This was referenced Dec 1, 2023

Error when disconnecting the wallet while the bridge activity is fetching threshold-network/token-dashboard#669

Open

UI: Header component #13

Merged

UI: PageLayout component #15

Merged

kpyszkowski added 5 commits December 5, 2023 16:03

Merge branch 'main' into main-cleanup

4e1477f

Merge branch 'feedback-removal' into main-cleanup

9f4e2ec

Merge branch 'staking-removal' into main-cleanup

a197135

Merge branch 'overview-removal' into main-cleanup

4601301

Merge branch 'main' into main-cleanup

710a811

kpyszkowski mentioned this pull request Dec 11, 2023

UI: Balance component #17

Merged

This was referenced Dec 21, 2023

UI: Deposit Details step #18

Merged

UI: Timeline component #19

Merged

UI: Provide Data step #20

Merged

This was referenced Jan 8, 2024

UI: Make Deposit step #21

Merged

UI: Initiate Minting step #22

Merged

michalsmiarowski mentioned this pull request Jan 10, 2024

Remove "Staking" page #10

Merged

michalsmiarowski requested changes Jan 10, 2024

View reviewed changes

michalsmiarowski mentioned this pull request Jan 10, 2024

Groundwork and repository setup #1

Closed

7 tasks

Merge branch 'main' into main-cleanup

ef56860

kpyszkowski requested a review from michalsmiarowski January 11, 2024 12:30

kpyszkowski marked this pull request as ready for review January 11, 2024 12:31

Remove redundant useEffect call

920b31d

Fetching ETH USD price is not needed in Base app

michalsmiarowski requested changes Jan 12, 2024

View reviewed changes

src/components/tBTC/BridgeActivity.tsx Outdated Show resolved Hide resolved

src/components/tBTC/BridgeActivity.tsx Outdated Show resolved Hide resolved

src/components/tBTC/BridgeActivity.tsx Outdated Show resolved Hide resolved

Fix BridgeActivity history conditional rendering

78f10f1

Updated conditions to include wallet connection. Fixed typo: `epmty` => `empty`

Remove isWalletConnected from context

cfb65f4

Adjust empty state rendering conditions.

kpyszkowski requested a review from michalsmiarowski January 12, 2024 17:15

michalsmiarowski requested changes Jan 15, 2024

View reviewed changes

src/components/tBTC/BridgeActivity.tsx Outdated Show resolved Hide resolved

Finetune empty state rendering condition

f568bab

Used dedicated boolean flag instead of the data itself

kpyszkowski requested a review from michalsmiarowski January 15, 2024 14:30

michalsmiarowski approved these changes Jan 15, 2024

View reviewed changes

michalsmiarowski merged commit afbd6b5 into main Jan 15, 2024

michalsmiarowski mentioned this pull request Jan 15, 2024

Clean-up v2 #27

Merged

7 tasks

michalsmiarowski added a commit that referenced this pull request Feb 14, 2024

Merge pull request #20 from threshold-network/ui/provide-data-step

83f0c00

UI: Provide Data step Ref: #2 Depends on: #12 This pull request introduces redesigned Provide Data minting flow step

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"Main" branch clean-up #12

"Main" branch clean-up #12

kpyszkowski commented Nov 30, 2023

kpyszkowski commented Dec 1, 2023 •

edited

Loading

michalsmiarowski left a comment

michalsmiarowski Jan 7, 2024 •

edited

Loading

michalsmiarowski Jan 11, 2024

michalsmiarowski Jan 7, 2024 •

edited

Loading

michalsmiarowski Jan 11, 2024

michalsmiarowski Jan 7, 2024

michalsmiarowski left a comment

michalsmiarowski left a comment

michalsmiarowski left a comment

"Main" branch clean-up #12

"Main" branch clean-up #12

Conversation

kpyszkowski commented Nov 30, 2023

kpyszkowski commented Dec 1, 2023 • edited Loading

michalsmiarowski left a comment

Choose a reason for hiding this comment

michalsmiarowski Jan 7, 2024 • edited Loading

Choose a reason for hiding this comment

michalsmiarowski Jan 11, 2024

Choose a reason for hiding this comment

michalsmiarowski Jan 7, 2024 • edited Loading

Choose a reason for hiding this comment

michalsmiarowski Jan 11, 2024

Choose a reason for hiding this comment

michalsmiarowski Jan 7, 2024

Choose a reason for hiding this comment

michalsmiarowski left a comment

Choose a reason for hiding this comment

michalsmiarowski left a comment

Choose a reason for hiding this comment

michalsmiarowski left a comment

Choose a reason for hiding this comment

kpyszkowski commented Dec 1, 2023 •

edited

Loading

michalsmiarowski Jan 7, 2024 •

edited

Loading

michalsmiarowski Jan 7, 2024 •

edited

Loading