Skip to content

Introduced data-table and confirm-dialog components: User#437

Merged
stefanoborini merged 7 commits into
masterfrom
user-view-with-toolkits
May 10, 2017
Merged

Introduced data-table and confirm-dialog components: User#437
stefanoborini merged 7 commits into
masterfrom
user-view-with-toolkits

Conversation

@stefanoborini

Copy link
Copy Markdown
Contributor

Change to Users page. Uses toolkit components data-table and confirm-dialog.

' v-if="newUserDialog.show"' +
' :show="newUserDialog.show"' +
' @created="newUserCreated"' +
' @closed="newUserDialogClosed"></new-user-dialog>' +

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.

@closed="newUserDialog.show = false" and remove the newUserDialogClosed method ? It will save 4 lines of code

' </data-table>' +
' <new-user-dialog ' +
' v-if="newUserDialog.show"' +
' :show="newUserDialog.show"' +

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.

maybe remove this line ?

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.

the new dialog needs to know if it's visible at the moment, so it needs that information. I am leaving the New dialogs unchanged for the moment, because I need a better dialog for them (and I am not sure it's worth extracting it)

@codecov-io

codecov-io commented May 10, 2017

Copy link
Copy Markdown

Codecov Report

Merging #437 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #437   +/-   ##
=======================================
  Coverage   95.33%   95.33%           
=======================================
  Files          88       88           
  Lines        4077     4077           
  Branches      259      259           
=======================================
  Hits         3887     3887           
  Misses        138      138           
  Partials       52       52

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2bdf22...28e1366. Read the comment docs.

@stefanoborini stefanoborini merged commit 076c4f7 into master May 10, 2017
@stefanoborini stefanoborini deleted the user-view-with-toolkits branch May 10, 2017 14:43
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.

3 participants