Skip to content

Annotations Crowdfunding UI.#558

Merged
poltak merged 10 commits intoWorldBrain:developfrom
digi0ps:annotations-crowdfunding
Sep 6, 2018
Merged

Annotations Crowdfunding UI.#558
poltak merged 10 commits intoWorldBrain:developfrom
digi0ps:annotations-crowdfunding

Conversation

@digi0ps
Copy link
Copy Markdown
Contributor

@digi0ps digi0ps commented Sep 3, 2018

Brought up a quick working modal for both CrowdFunding Box and Overview Modal.

  • Fix issue with Modal Closing

Forked from #541. Thanks to @pranshuchittora for his work (# on icons and refactoring Annotations CSS.

Related to #540 #552.

@poltak
Copy link
Copy Markdown
Member

poltak commented Sep 5, 2018

I really don't like the way 77ca94e solves the modal closing issue, mainly because it's now accessing the real DOM from within the React DOM, but also because it's coupling the crowdfunding modal-related logic with the sidebar stuff.
I played around with moving this check to native React DOM refs, although it seems quite difficult without refactoring how the sidebar feature's code is laid out. Let's go with it for now, as the main thing is it works from a user POV but hopefully we can address it in the near future.

@oliversauter @digi0ps Are the main remaining things now (at least to address #540):

  • setting up the link to push the user to if they click "Learn more"
  • modal + share button for collections (is anyone working on this, or planning to?)
  • analytics

anything else?

@blackforestboi
Copy link
Copy Markdown
Member

blackforestboi commented Sep 5, 2018

don't fully get what the problem with the modal closing is? works for me quiet well?
Can you explain the problem and how you wanted to solve it?

Problems I found:

  • in the overview, with a sidebar opened, it doesnt close anymore when clicking outside, and it does not switch to the annotations of another page when clicking on another annotation icon.
    Error: screen shot 2018-09-05 at 17 59 25

@mukeshkharita can you work on the analytics here, and add analytics to the following actions:

  • clicking on "reply" or "share" button
  • clicking on "Learn more button"

@digi0ps
Copy link
Copy Markdown
Contributor Author

digi0ps commented Sep 5, 2018

I really don't like the way 77ca94e solves the modal closing issue, mainly because it's now accessing the real DOM from within the React DOM, but also because it's coupling the crowdfunding modal-related logic with the sidebar stuff.

@poltak Yeah, not the best way to prevent the closing of sidebar. But the two were in very different modules, using refs would involve lot of passing down of props. So had to go for this way.

@poltak poltak force-pushed the annotations-crowdfunding branch from 3f6937a to 0062d2f Compare September 6, 2018 06:24
@poltak poltak force-pushed the annotations-crowdfunding branch from 0062d2f to 0bffb89 Compare September 6, 2018 07:27
- also moved to TS
- these two components duplicate a lot of code; it would be nice to DRY them out eventually
- also fixed up crowdfunding button defined check
@poltak poltak merged commit 836d8fb into WorldBrain:develop Sep 6, 2018
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.

5 participants