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.

Sign up for GitHub

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

Jump to bottom

ui: Hide old errors on market page forms #1662

Merged
merged 1 commit into from
Jun 17, 2022
Merged

ui: Hide old errors on market page forms #1662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions client/webserver/site/src/js/dexsettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ export default class DexSettingsPage extends BasePage {

const closePopups = () => {
Doc.hide(page.forms)
page.exportSeedPW.value = ''
page.seedDiv.textContent = ''
}

Doc.bind(page.forms, 'mousedown', (e: MouseEvent) => {
Expand Down
4 changes: 3 additions & 1 deletion client/webserver/site/src/js/forms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,12 +855,14 @@ export class UnlockWalletForm {
bind(form, this.page.submitUnlock, () => this.submit())
}

setAsset (asset: SupportedAsset) {
refresh (asset: SupportedAsset) {
const page = this.page
this.currentAsset = asset
page.uwAssetLogo.src = Doc.logoPath(asset.symbol)
page.uwAssetName.textContent = asset.info.name
page.uwAppPass.value = ''
page.unlockErr.textContent = ''
Doc.hide(page.unlockErr)
const hidePWBox = State.passwordIsCached() || (this.pwCache && this.pwCache.pw)
if (hidePWBox) Doc.hide(page.uwAppPassBox)
else Doc.show(page.uwAppPassBox)
Expand Down
16 changes: 12 additions & 4 deletions client/webserver/site/src/js/markets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ export default class MarketsPage extends BasePage {
bindForm(page.cancelForm, page.cancelSubmit, async () => { this.submitCancel() })
// Order detail view
Doc.bind(page.vFeeDetails, 'click', () => this.showForm(page.vDetailPane))
Doc.bind(page.closeDetailPane, 'click', () => this.showForm(page.verifyForm))
Doc.bind(page.closeDetailPane, 'click', () => this.showVerifyForm())
// Bind active orders list's header sort events.
page.liveTable.querySelectorAll('[data-ordercol]')
.forEach((th: HTMLElement) => bind(th, 'click', () => setOrdersSortCol(th.dataset.ordercol || '')))
Expand Down Expand Up @@ -388,7 +388,7 @@ export default class MarketsPage extends BasePage {

// If the user clicks outside of a form, it should close the page overlay.
bind(page.forms, 'mousedown', (e: MouseEvent) => {
if (Doc.isDisplayed(page.vDetailPane) && !Doc.mouseInElement(e, page.vDetailPane)) return this.showForm(page.verifyForm)
if (Doc.isDisplayed(page.vDetailPane) && !Doc.mouseInElement(e, page.vDetailPane)) return this.showVerifyForm()
if (!Doc.mouseInElement(e, this.currentForm)) {
closePopups()
}
Expand Down Expand Up @@ -1354,7 +1354,7 @@ export default class MarketsPage extends BasePage {
const page = this.page
this.openAsset = asset
this.openFunc = f
this.unlockForm.setAsset(app().assets[asset.id])
this.unlockForm.refresh(app().assets[asset.id])
this.showForm(page.unlockWalletForm)
page.uwAppPass.focus()
}
Expand Down Expand Up @@ -1428,7 +1428,7 @@ export default class MarketsPage extends BasePage {
page.vSubmit.classList.add(buyBtnClass)
page.vSubmit.classList.remove(sellBtnClass)
}
this.showForm(page.verifyForm)
this.showVerifyForm()
page.vPass.focus()

if (baseAsset.wallet.open && quoteAsset.wallet.open) this.preOrder(order)
Expand All @@ -1439,6 +1439,13 @@ export default class MarketsPage extends BasePage {
}
}

/* showVerifyForm displays form to verify an order */
async showVerifyForm () {
const page = this.page
Doc.hide(page.vErr)
this.showForm(page.verifyForm)
Comment on lines +1445 to +1446
Copy link
Member

Choose a reason for hiding this comment

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

Strikes me as a bit backwards to clean up when opening the form instead of when closing the form.

Would the following not be targeted well enough?

diff --git a/client/webserver/site/src/js/markets.ts b/client/webserver/site/src/js/markets.ts
index f7a791e0..c4a1ff03 100644
--- a/client/webserver/site/src/js/markets.ts
+++ b/client/webserver/site/src/js/markets.ts
@@ -381,8 +381,9 @@ export default class MarketsPage extends BasePage {
     setOrdersSortColClasses()
 
     const closePopups = () => {
-      Doc.hide(page.forms)
+      Doc.hide(page.forms, page.vErr)
       page.vPass.value = ''
+      page.vErr.textContent = ''
       page.cancelPass.value = ''
     }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both of these work, but I was thinking of it more of setting up the form to be displayed instead of cleaning it up. I actually see now that all of the forms on this page have the same issue, so I'll update those as well.

For a lot of the forms we have a refresh method that sets up the form. How about we make it the standard from now on to always have a refresh function for every popup form which must be called whenever you open it? Then, when forms are reused, these cleanup things don't have to be repeated in the closePopups function of every page. These closePopups methods can get very big when you need to handle the cleanup for each popup on the page each time you close one of them.

}

/*
* submitEstimateUnlock reads the current vUnlockPass and unlocks any locked
* wallets.
Expand Down Expand Up @@ -1611,6 +1618,7 @@ export default class MarketsPage extends BasePage {
const asset = OrderUtil.isMarketBuy(order) ? this.market.quote : this.market.base
page.cancelRemain.textContent = Doc.formatCoinValue(remaining, asset.info.unitinfo)
page.cancelUnit.textContent = asset.symbol.toUpperCase()
Doc.hide(page.cancelErr)
this.showForm(page.cancelForm)
page.cancelPass.focus()
this.cancelData = {
Expand Down
2 changes: 1 addition & 1 deletion client/webserver/site/src/js/wallets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export default class WalletsPage extends BasePage {
const page = this.page
this.openAsset = this.lastFormAsset = assetID
await this.hideBox()
this.unlockForm.setAsset(app().assets[assetID])
this.unlockForm.refresh(app().assets[assetID])
if (errorMsg) this.unlockForm.showErrorOnly(errorMsg)
this.animation = this.showBox(page.unlockWalletForm, page.walletPass)
}
Expand Down