-
Notifications
You must be signed in to change notification settings - Fork 136
fix for issue 2180 #2957
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
fix for issue 2180 #2957
Conversation
maxceem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @sumitdaga
Most things work good, there are two moments which I've noticed:
-
Make the topic background "orange" as in design.
-
Private topics are shown to the
pshah_customeruser https://monosnap.com/file/wl8Nj5S9cG0ldc9517F4revU4Q6iYi even thoughcanAccessPrivatePostsisfalsehere https://github.com/appirio-tech/connect-app/pull/2957/files#diff-cd301f6eb8ee6da3f365fab832384e3dR381
Thank you.
|
@sumitdaga just in case would like to remind about this PR as the Bug Bash challenge in review phase now, which will be finished in 1.5 days. |
|
@maxceem |
It would be great if not complicated but is not required. |
|
Ah, during editing it should be orange. During creating would be also good, but not required. |
|
@maxceem also should the lock icon be present during editing ? |
|
@sumitdaga no, I think better to keep space for the fields. The color would be enough. |
you mean make it orange when the private switch is toggled to on ...it may look odd! |
|
@maxceem please take a look now! |
yeah, it feels that it would make it more easy to track if it's private or not and reduce mistakes. But as it wasn't required initially, it can be skipped. |
|
@maxceem |
maxceem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for changing color during topic creation @sumitdaga
Great work here has been done!
| if (checkPermission(PERMISSIONS.ACCESS_PRIVATE_POST)) { | ||
| allFeed = [...allFeed, ...projectTopics.feeds[PROJECT_FEED_TYPE_MESSAGES].topics] | ||
| } | ||
| const allFeedCount = projectTopics.feeds[PROJECT_FEED_TYPE_PRIMARY].totalCount + projectTopics.feeds[PROJECT_FEED_TYPE_MESSAGES].totalCount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maxceem
allFeedCount needs to be updated as well (checkPermission) ...although i don't think feedTotalCount is used anywhere!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @sumitdaga, fixed it 0277743

No description provided.