-
Notifications
You must be signed in to change notification settings - Fork 34
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
Issue/1575 omit already shared #1918
Issue/1575 omit already shared #1918
Conversation
merging issue into dev branch
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.
This functions well, there are just a few minor nits to pick as far as cleaning up the code and making sure the tests are actually testing.
packages/app/obojobo-repository/shared/components/module-permissions-dialog.jsx
Outdated
Show resolved
Hide resolved
packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js
Outdated
Show resolved
Hide resolved
packages/app/obojobo-repository/shared/components/module-permissions-dialog.test.js
Show resolved
Hide resolved
packages/app/obojobo-repository/shared/components/people-search-dialog.jsx
Outdated
Show resolved
Hide resolved
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.
Looks good to me.
Just remove the large commented block from packages/app/obojobo-repository/shared/components/people-search-dialog.jsx
and it should be all set.
packages/app/obojobo-repository/shared/components/people-search-dialog.jsx
Outdated
Show resolved
Hide resolved
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep our backlog under control, but we thank you for your contributions. |
d30da5a
to
048816c
Compare
I merged in dev 30 except four style files which have lint errors. These lint errors appear to be already in dev30, so if I alter the files in order to commit them then they will continue to show up on this PR diff. I was not sure if that was ideal so I didn't not commit those files yet. They show up in the diff right now without the dev30 merge updates. |
9155352
to
048816c
Compare
merged with Dev 30 and lint style updates made in 4 files. These four files more-info-box.scss, objective-list-view.scss, search.scss, and objective-item.scss are seperate from this PR, but needed their styles to be updated to be merged in. I also removed the commented code requested to be removed above. |
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.
Code isn't building to test, see below.
.../app/obojobo-document-engine/src/scripts/oboeditor/components/objectives/objective-item.scss
Show resolved
Hide resolved
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.
Looks like this is working properly and performing as advertised.
When sharing omits users that already have access