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

NT-1910: UX – Not logged in / Not backer for Comment Composer #1271

Merged
merged 3 commits into from Jun 3, 2021

Conversation

leighdouglas
Copy link
Contributor

@leighdouglas leighdouglas commented Jun 3, 2021

πŸ“² What

This change handles the different states of the comment composer in relation to the user state i.e. logged out, not backing, etc.
Also had to update the UI to make the separator disappear as it was still appearing when the composer was gone.

πŸ€” Why

There are three states -> ENABLED, DISABLED, GONE

  • GONE -> The comment composer should be completely gone if the user is not logged in.
  • ENABLED -> If the user is logged in and a backer, or is the creator, the comment composer is enabled.
  • DISABLED -> If the user is logged in but not a backer, then the comment composer should be in the disabled state

πŸ›  How

Created an enum that holds these three states, and when the comment view model is initiated, it emits this state to the activity and the view responds accordingly.

πŸ‘€ See

Logged in: Backing and not backing
Screenshot_1622673686 Screenshot_1622673709

Logged out:
Screenshot_1622673778

πŸ“‹ QA

Make sure you have a project that you are not backing, a project that you are backing, and a project that you created, and all projects must have comments. Test going to the comments directly from the project page, as well as from the updates page. Then test the different states listed below.

πŸ”΄ = no comment composer
🟑 = comment composer disabled
🟒 = comment composer enabled

These are the different states to test:
Logged out = πŸ”΄
Logged in and not backing = 🟑
Logged in and backing = 🟒
Logged in and creator = 🟒

Story πŸ“–

NT-1910: UX – Not logged in / Not backer for Comment Composer

@codecov
Copy link

codecov bot commented Jun 3, 2021

Codecov Report

Merging #1271 (7b7fa6f) into master (f7c7046) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1271   +/-   ##
=========================================
  Coverage     74.34%   74.34%           
  Complexity      739      739           
=========================================
  Files           221      221           
  Lines          6646     6646           
  Branches        405      405           
=========================================
  Hits           4941     4941           
  Misses         1569     1569           
  Partials        136      136           

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 f7c7046...7b7fa6f. Read the comment docs.

@leighdouglas leighdouglas changed the title Leigh/nt 1910 comment composer gone NT-1910: UX – Not logged in / Not backer for Comment Composer Jun 3, 2021
@leighdouglas leighdouglas marked this pull request as ready for review June 3, 2021 18:00
@leighdouglas leighdouglas force-pushed the leigh/nt-1910-comment-composer-gone branch from 7f2816d to 7b7fa6f Compare June 3, 2021 18:07
Copy link
Contributor

@Arkariang Arkariang left a comment

Choose a reason for hiding this comment

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

It looks great @leighdouglas !

@leighdouglas
Copy link
Contributor Author

@Arkariang I noticed that too and figured we could update in a subsequent ticket. I'll make a ticket for it :)

@leighdouglas leighdouglas merged commit 6bd99a5 into master Jun 3, 2021
@leighdouglas leighdouglas deleted the leigh/nt-1910-comment-composer-gone branch June 3, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants