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.

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

ACT-666 Re-implement Documents using Vue: Create modal #446

Merged
merged 5 commits into from
Apr 17, 2020
Merged

Conversation

chelliah
Copy link
Contributor

@chelliah chelliah commented Apr 9, 2020

What is the Purpose?

This PR reimplements the 'Add new document' modal using the Vue.js vue_modal component

What was the approach?

I used the 'indicator_list.html' and 'add_indicator_modal.html' as the base model and copied the same logic into the documents_list file

Are there any concerns to addressed further before or after merging this PR?

What would the next steps be? I could implement document deletion, or move the rest of the documention_list page to vue.

Mentions?

Mention persons you'd like to review this PR

Issue(s) affected?

(https://github.com/hikaya-io/activity/issues/311)

@codecov-io
Copy link

codecov-io commented Apr 9, 2020

Codecov Report

Merging #446 into dev will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #446   +/-   ##
=======================================
  Coverage   50.60%   50.60%           
=======================================
  Files         110      110           
  Lines        8359     8359           
=======================================
  Hits         4230     4230           
  Misses       4129     4129           
Impacted Files Coverage Δ
workflow/views.py 30.31% <0.00%> (ø)

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 1e9eec9...cfd407e. Read the comment docs.

@Kimaiyo077
Copy link
Collaborator

@chelliah , Nice work on implementing the add document modal with Vue. Your work looks great but I had a few observations. One is, there seems to be an error that pops up before the component is mounted. Could you look into why this is happening?

Screenshot (37)

That error means that I cannot get Programs to populate the options in the modal itself even though I have existing programs in my organization.

Screenshot (38)

Please look into these two issues so I can be able to test whether I create a new document

odenypeter
odenypeter previously approved these changes Apr 9, 2020
Copy link
Contributor

@odenypeter odenypeter left a comment

Choose a reason for hiding this comment

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

Looks good if the ticket is only for Document creation 👍
Just address @Kimaiyo077 request

@odenypeter odenypeter dismissed their stale review April 9, 2020 09:25

Waiting Isaacs comment

@chelliah
Copy link
Contributor Author

chelliah commented Apr 9, 2020

Thanks @Kimaiyo077! I'll take a look at that loading error and let you know

@andrewtpham andrewtpham linked an issue Apr 9, 2020 that may be closed by this pull request
@andrewtpham
Copy link
Member

Latest comment from @chelliah:

basically, I experimented with the programs list to see if I could recreate the bug, but wasn't able to get too far. I wasn't sure if this was a client-side issue, or some edge case in which one of the programs had funny data which was causing things to fail

@Kimaiyo077 is there any support you can provide on this?

Copy link
Member

@ninetteadhikari ninetteadhikari left a comment

Choose a reason for hiding this comment

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

@chelliah This is working from my end 😊 🧁
@Kimaiyo077 I couldn't reproduce your issue :( shall we merge this?

@ninetteadhikari ninetteadhikari merged commit d2e147e into dev Apr 17, 2020
@ninetteadhikari ninetteadhikari deleted the ACT-666 branch April 17, 2020 16:01
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.

ACT-666 Re-implement Documents using Vue: Create modal
6 participants