Skip to content
This repository was archived by the owner on May 24, 2021. It is now read-only.

Conversation

@rowandh
Copy link
Collaborator

@rowandh rowandh commented Mar 14, 2019

  • Adds configurable baseunit
  • We don't currently store any preferences locally, so I used localStorage. Electron seems to support this fine but we can change it if needed.

See 3091.

TODO:

  • There's a couple of spots where the base unit still needs to be added

@rowandh rowandh changed the title Base unit [draft] Add coin base unit [draft] Mar 14, 2019
@dev0tion
Copy link
Collaborator

Nice work, ping me if you want a review.
Does using localStorage keep the settings between sessions?

@rowandh
Copy link
Collaborator Author

rowandh commented Mar 14, 2019

It does appear to, yes.

constructor(private globalService: GlobalService) {
this.baseUnitSubscription = this.globalService.baseUnit.subscribe(b => {
this.baseUnit = b;
this.format = this.format != undefined ? this.format : b.defaultFormat;
Copy link
Contributor

Choose a reason for hiding this comment

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

To be safe, I would go with !!this.format ? this.format : b.defaultFormat instead of this.format != undefined ? this.format : b.defaultFormat

let temp;
if (typeof value === 'number') {
temp = value / 100000000;
temp = value / multiple;
Copy link
Contributor

Choose a reason for hiding this comment

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

You might need to check that multiple != 0 to avoid division by 0

this.setApiPort();

// Store by name so we can match the object and populate the settings list correctly.
let storedBaseUnitName = localStorage.getItem('baseUnit');
Copy link
Contributor

Choose a reason for hiding this comment

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

storedBaseUnitName doesn't change, use const instead of let

this.coinUnit = coinUnit;
}

setBaseUnit(baseUnit: BaseUnit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably use property get and set to simplify this:

set baseUnit(value: BaseUnit) {
    localStorage.setItem('baseUnit', baseUnit.name);
    this.baseUnit.next(baseUnit);
}

get baseUnits() {
    return this.baseUnits;
}

@@ -0,0 +1,16 @@
<div class="card p-4">
<div class="card-header">Change Base Unit</div>
<div class="card-body" style="min-height:200px;">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move all inline styles into a base-unit.component.scss

<select class="custom-select col-12" [(ngModel)]="selectedBaseUnit" [ngModelOptions]="{standalone: true}" (change)="onBaseUnitChanged()">
<option *ngFor="let baseUnit of baseUnits" [ngValue]="baseUnit">{{ coinUnit | prefixCoinUnit:baseUnit.name }}</option>
</select>
<span style="padding-left: 15px;">1 Strat = <coins [amount]="100000000"></coins></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move all inline styles into a base-unit.component.scss

<td class="text-left" *ngIf="transaction.transactionType == 'sent'"><strong>- {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'received'"><strong>+ {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'staked'"><strong>+ {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'sent'"><strong>- <coins [amount]="transaction.transactionAmount + transaction.transactionFee"></coins></strong></td>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably move transaction.transactionAmount + transaction.transactionFee into a variable or a property to .ts file, it is used in few places

<td class="text-left" *ngIf="transaction.transactionType == 'sent'"><strong>- {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'received'"><strong>+ {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'staked'"><strong>+ {{ transaction.transactionAmount + transaction.transactionFee | coinNotation }} {{ coinUnit }}</strong></td>
<td class="text-left" *ngIf="transaction.transactionType == 'sent'"><strong>- <coins [amount]="transaction.transactionAmount + transaction.transactionFee"></coins></strong></td>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably move transaction.transactionAmount + transaction.transactionFee into a variable or a property to .ts file, it is used in few places

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants