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

Jump to bottom

User and groups control panel #608

Merged
merged 57 commits into from Sep 26, 2019
Merged

User and groups control panel #608

merged 57 commits into from Sep 26, 2019

Conversation

nileshgulia1
Copy link
Member

@nileshgulia1 nileshgulia1 commented Feb 9, 2019 •
edited

#591 @csenger already implemented users control panel i.e ability to add and search for users .
Implementations:

  • Now we have the ability to add and display group name with users.
  • Ability to Delete Groups.
  • Ability to Search for Groups.

Left overs:

  • Ability to add members into groups (possibly by clicking one of the created groups, there should be pop up for adding users (like in original Plone). Although I'm not clear how to group these members together because it don't see an endpoint in the schema, need discussion from members).

@mister-roboto
Copy link

@mister-roboto mister-roboto commented Feb 9, 2019

@nileshgulia1 thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@nileshgulia1 nileshgulia1 added this to In progress in Nilesh Feb 9, 2019
@sneridagh
Copy link
Member

@sneridagh sneridagh commented Feb 27, 2019 •
edited

@nileshgulia1

We need it to match the look and feel in:
https://app.zeplin.io/project/5c76b54e0e8c876239917ea3/screen/5c76b59686d8b86292eecca7

Please, can you take care? The screens are mainly in mobile sizes, but easy to translate to big ones. For the tables, very important: make them responsive, so the overflow (if any) is hidden and scrollable horizontally. Like https://getbootstrap.com/docs/4.0/content/tables/#responsive-tables Notice the subtle shadow in the mocks, Albert to blame!

BTW, also, all content area in Volto is enclosed into a Container (for responsiveness), so make sure it is.

Also, I've noticed that the modal is kind of broken (it's displaced to the bottom).

Thanks!

@sneridagh
Copy link
Member

@sneridagh sneridagh commented Feb 27, 2019

More, the password in the field is shown in clear text.
The search does not search on type.

@tisto
Copy link
Member

@tisto tisto commented Sep 22, 2019

@nileshgulia1 thank you Nilesh!

Volto 4 Final automation moved this from In progress to Reviewer approved Sep 22, 2019
tisto
tisto approved these changes Sep 22, 2019
View changes
Copy link
Member

@tisto tisto left a comment

There are still details we have to improve. Though, the base functionality is there. Therefore I'd say we should merge now.

tisto
tisto reviewed Sep 22, 2019
View changes
CHANGELOG.md Outdated
@@ -528,6 +528,15 @@
- Drag and drop to reorder tiles @robgietema
- Enhanced DraftJS AnchorLink Plugin @robgietema @sneridagh
- Added the configuration required in Webpack config to load CSS modules in the project, required by DraftJS AnchorLink plugin @sneridagh
* Refactor UserControlPanel add delete and add user actions @csenger
Copy link
Member

@tisto tisto Sep 22, 2019

Choose a reason for hiding this comment

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

@nileshgulia1 This does not seem right. Can you re-add your changelog to the latest unreleased block on the changelog?

Copy link
Member Author

@nileshgulia1 nileshgulia1 Sep 22, 2019

Choose a reason for hiding this comment

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

@tisto I have added the changelog!

Copy link
Member

@tisto tisto Sep 22, 2019

Choose a reason for hiding this comment

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

@nileshgulia1 you still have to remove those duplicate changelog entries here.

Copy link
Member Author

@nileshgulia1 nileshgulia1 Sep 22, 2019 •
edited

Choose a reason for hiding this comment

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

Sorry. I removed the old entries now. :)

@tisto
Copy link
Member

@tisto tisto commented Sep 25, 2019

@nileshgulia1 when you merged master into your branch you introduced two new checks that are now failing on Travis:

  1. eslint warnings: we fail the build on warnings now. check the output of "yarn lint" locally and fix the problems indicated
  2. i18n warnings: we fail the build when the i18n is outdated, just run "yarn i18n" locally and check in the changes, this should fix that problem on Travis.

@nileshgulia1 nileshgulia1 force-pushed the feature-29-user-control-panel branch from a63b8c9 to 6c01077 Compare Sep 25, 2019
@nileshgulia1
Copy link
Member Author

@nileshgulia1 nileshgulia1 commented Sep 25, 2019

@tisto Should I commit the result of yarn i18n?

@tisto
Copy link
Member

@tisto tisto commented Sep 25, 2019

@nileshgulia1 yes, please.

@tisto
Copy link
Member

@tisto tisto commented Sep 25, 2019

@nileshgulia1 I updated the i18n. Let's see what the next build will return.

@nileshgulia1
Copy link
Member Author

@nileshgulia1 nileshgulia1 commented Sep 25, 2019

@tisto All Green!

tisto
tisto reviewed Sep 26, 2019
View changes
locales/de/LC_MESSAGES/volto.po
@@ -26,6 +26,14 @@ msgstr ""
"Preferred-Encodings: utf-8 latin1\n"
"X-Is-Fallback-For: de-at de-li de-lu de-ch de-de\n"

#: components/manage/Controlpanels/UsersControlpanel
msgid "ADD NEW GROUP"
Copy link
Member

@tisto tisto Sep 26, 2019

Choose a reason for hiding this comment

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

@nileshgulia1 the message string should not be all upper-case. You can make the string appear all upper-case with CSS.

tisto
tisto reviewed Sep 26, 2019
View changes
.gitignore
@@ -15,7 +15,8 @@ build
.DS_Store
.idea
lighthouse-report.html
.vscode/
Copy link
Member

@tisto tisto Sep 26, 2019

Choose a reason for hiding this comment

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

@nileshgulia1 you might want to re-add this line here for vscode users.

@tisto tisto merged commit 0f58427 into master Sep 26, 2019
6 checks passed
Volto 4 Final automation moved this from Reviewer approved to Done Sep 26, 2019
@tisto tisto deleted the feature-29-user-control-panel branch Sep 26, 2019
@tisto
Copy link
Member

@tisto tisto commented Sep 26, 2019

@nileshgulia1 thank you for your work on this! Merged. Would you mind creating another PR with the two small fixes I told you about!

@nileshgulia1
Copy link
Member Author

@nileshgulia1 nileshgulia1 commented Sep 26, 2019

@tisto okay! I'll do it asap.

@nileshgulia1 nileshgulia1 mentioned this pull request Sep 26, 2019
Merged
sneridagh added a commit that referenced this issue Sep 27, 2019
@sneridagh
Merge branch 'master' into add-missing-translations
287977b 
* master:
  change name of metadata and properties (#893)
  Move the Tile Edit wrapper one level up to the tiles engine (#895)
  Cypress create content type (#897)
  Fix file upload acceptance test (#882)
  Rework users controlpanel (#891)
  Dependencies upgrades (router, redux) (#890)
  User and groups control panel (#608)
  Add animated gifs for Volto table and toolbar to the docs.
  Do not run createContent cypress test on Guillotina.
  Use forEach instead of map in BodyClass.jsx.
  Remove unused getSpecialList and comment out unused ReferenceWidget import.
  Fail eslint check on any warnings.
  Add createContent cypress keyword.
  Fix typo in login.js acceptance test.
  Update the acceptance test readme.
@nileshgulia1 nileshgulia1 mentioned this pull request Mar 4, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tisto tisto

Assignees

@nileshgulia1 nileshgulia1

Labels
22 status: in-progress
Projects
No open projects
Nilesh
  
In progress
Volto 4 Final
  
Done
Milestone
4.0.x
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nileshgulia1 @mister-roboto @sneridagh @tisto @robgietema @csenger