Skip to content
This repository has been archived by the owner on Nov 4, 2021. It is now read-only.

Improvements and fixes #40

Merged
merged 14 commits into from
Feb 20, 2017
Merged

Improvements and fixes #40

merged 14 commits into from
Feb 20, 2017

Conversation

ferni
Copy link
Contributor

@ferni ferni commented Feb 18, 2017

  • Fix two bugs related to content failing to update between searches.
  • New components_address template, which adds the identicon and turns green if the user owns it.
  • Remove EthAccounts which is not needed.
  • Upgrade ethereum-ens.js to 0.4.2 (remember to meteor npm up)

registrar.unsealBid(bid, {
from: mainAccount,
from: web3.eth.accounts[0], // Any account can reveal
Copy link
Collaborator

Choose a reason for hiding this comment

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

always checks if it exists!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's done at the top of the function.

let bid = template.data.bid.bid ? template.data.bid.bid : template.data.bid;
MyBids.update({ _id: bid._id }, { $set: {revealing: true} });

TemplateVar.set(template, 'revealing', true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

add the name to the template variable name, 'revealing-'+name. This way, if the user clicks on another name while it waits for it to be revealed, the next page won't be in the same state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't TemplateVar supposed to be template instance specific? I'll apply the suggested change anyway because why not.

@@ -13,6 +13,13 @@ Template['components_nameStatus'].onCreated(function() {
try {
registrar.getEntry(name, (err, entry) => {
if(!err && entry) {
let prevInfo = TemplateVar.get(template, 'nameInfo');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this redundant with line 40?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's stuff below that line that would be undesirable to run unless the name or status changes (The resetting of the TemplateVars, which cause a flicker if they're reset every second).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have a point in that maybe both checks could be unified. I'm not liking how this piece of code is shaping up, I feel it needs some reorganization. I think the home logic could be put in a separate template called "home", and set entry.mode to "home" when !name. And as a nice side-effect, you would also see the home screen when deleting the name with backspace.

@@ -34,7 +33,7 @@ Template['status-auction'].events({
}
console.log('secret', secret);

if (accounts.length == 0) {
if (web3.eth.accounts.length == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If .accounts doesn't exist, checking it's length will return an error

@ferni ferni merged commit 79537be into master Feb 20, 2017
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.

2 participants