Skip to content

minor tweaks @ Repo.vue#18

Merged
stefanjudis merged 1 commit intoforrest-app:masterfrom
stryju:patch-1
Jul 23, 2016
Merged

minor tweaks @ Repo.vue#18
stefanjudis merged 1 commit intoforrest-app:masterfrom
stryju:patch-1

Conversation

@stryju
Copy link
Copy Markdown
Contributor

@stryju stryju commented Jul 23, 2016

some minor tweaks (including one style-tweak) ;-)

  • some micro-optimizations (cache, early exit)
  • minor styling tweaks

some minor tweaks (including one style-tweak) ;-)
@stryju stryju changed the title Update Repo.vue minor tweaks @ Repo.vue Jul 23, 2016
},

data() {
const repoName = decodeURIComponent( this.repoName );
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cache the potentially "expensive" value

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

jap... totally with you here. :)

@stefanjudis
Copy link
Copy Markdown
Member

👍

@stefanjudis stefanjudis added this to the v1.0.0-beta milestone Jul 23, 2016
@stefanjudis stefanjudis self-assigned this Jul 23, 2016
@stefanjudis stefanjudis merged commit dd9ff93 into forrest-app:master Jul 23, 2016
if ( index > 0 ) {
this.scriptElements[ index - 1 ].focus();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

for the early escape approach, but I'll revert this parts of this change in master again.

Because

if ( index > 0 ) {
  this.scriptElements[ index - 1 ].focus();
}

this is still needed. Currently it's throwing... I missed that one too.

Logic should be, if the current focused element is not the first one, then focus the one before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oooh!

@stryju stryju deleted the patch-1 branch July 24, 2016 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants