Skip to content

#171784041 Build the UI for User Role Settings #8

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

fordarnold
Copy link
Collaborator

@fordarnold fordarnold commented May 28, 2020

Role Settings UI

@fordarnold fordarnold self-assigned this May 28, 2020
@fordarnold fordarnold added the WIP Work in progress label May 28, 2020
@irfiacre irfiacre temporarily deployed to barefoot-ui-staging June 6, 2020 09:53 Inactive
@irfiacre irfiacre temporarily deployed to barefoot-ui-staging June 6, 2020 11:32 Inactive
@irfiacre irfiacre temporarily deployed to barefoot-ui-staging June 6, 2020 11:43 Inactive
@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch 2 times, most recently from 74b6940 to d93d282 Compare June 7, 2020 20:00
@fordarnold fordarnold added the Ready For Review Requires code review from Dev team label Jun 8, 2020
@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch 2 times, most recently from 8e0cb8a to c3204d0 Compare June 11, 2020 07:30
@irfiacre
Copy link
Collaborator

irfiacre commented Jun 11, 2020

Nice work @fordarnold

However on the profile(sideBar), maybe if you could use the background color which is white, and highlight the active page. Plus place the icons as they should be, I think it would be better.

For the navBar, I like that you added some other links, maybe you could add also the signout icon to allow a user to sign out and space them accordingly.
image

In general, you could also use the designed background-color: #083B66 or from the _settings.scss: $baseBlue.

There is a way you may use the Figma icons
image

Just copy the code as svg and the paste the in a filename.svg to get the icon.

@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch 3 times, most recently from 4ee25e4 to ffbeacc Compare June 16, 2020 17:30
@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch from ffbeacc to e112d19 Compare June 22, 2020 06:24
@fordarnold fordarnold added enhancement New feature or request and removed WIP Work in progress labels Jun 22, 2020
@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch from e112d19 to 3f41a9c Compare June 22, 2020 19:12
@fordarnold fordarnold added the TTL Review Requires code review by Technical Team Lead label Jun 23, 2020
Copy link
Collaborator

@benshidanny11 benshidanny11 left a comment

Choose a reason for hiding this comment

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

Good job

@fordarnold
Copy link
Collaborator Author

For the navBar, I like that you added some other links, maybe you could add also the signout icon to allow a user to sign out and space them accordingly.

@irfiacre I believe the person working on the User Sign-Out feature should be responsible for adding the icon, functionality and tests. What do you think?

@fordarnold
Copy link
Collaborator Author

In general, you could also use the designed background-color: #083B66 or from the _settings.scss: $baseBlue.

Done 👍🏽

@fordarnold
Copy link
Collaborator Author

There is a way you may use the Figma icons...
Just copy the code as svg and the paste the in a filename.svg to get the icon.

Thanks for this great advice. I'm done implementing the icons.

- added config file for Code Climate
- setup dashboard page-layout structure
- added sample landing page for dashboard
- added page for user Roles and Permissions
- setup routes for existing dashboard pages
- setup dashboard Header design
- load dashboard UI styles
- setup dashboard Footer design
- load dashboard font styles
- setup dashboard Sidebar design
- added main content for Roles page
- implemented CRUD functionality for Roles API
- setup unit/integration tests
- fix code coverage issues
- work on PR review feedback

Don't confuse 'Sidebar' and 'Sidenav'.
- Sidebar is for desktop view.
- Sidenav is for mobile view.

[Finishes #171784041]
@fordarnold fordarnold force-pushed the ft-ui-user-roles-171784041 branch from 3f41a9c to 1ea8fe1 Compare June 24, 2020 09:33
@bdushimi
Copy link
Collaborator

bdushimi commented Jun 24, 2020

Hey, @fordarnold good work so far. I have a few feedback/comments regarding the visual aspects.

  1. The footer is not at the bottom of the page as it is supposed to be.
  2. When signed in with the following credentials [Email: [email protected] Password: user@123], clicking on the View User Roles results into a blank page and errors in the console.
  3. The UI is not yet responsive.
  4. This route /dashboard should be protected, but it is not i.e. it can be accessed with or without a valid token. The expected behavior is redirection to the login page in the event there's no valid token.
  5. The Make Trip Request link is not functional. It is supposed to be functional since Francois's PR/Make Trip Request feature has been merged.
  6. Logged in as the admin, there's an excess of roles duplication. some role cards don't have edit & delete buttons
  7. Logged in as the admin and edit a role:
    1. Before saving the changes, the system asks a role name which should not be the case.
    2. The save-changes and close-without-saving buttons should be aligned horizontally. Optionally centered.
    3. Let the save-button be active if and only if there is an updated checkbox and the close-without-changes be always active.
    4. Once a particular role's permissions changed/updated, upon the page refresh the changes are not reflected anymore.
    5. All roles have the same permissions
  8. When a user is about to delete a particular role, I suggest to apply a background of red color on the YES, DELETE button and also give the NO, CANCEL as a background color as well.

I think it would be better to review the source codes after fixing the UI/functionality aspects of the feature.

Screen Shot 2020-06-24 at 15 55 30
Screen Shot 2020-06-24 at 15 42 15
Screen Shot 2020-06-24 at 15 42 22
Screen Shot 2020-06-24 at 15 35 36
Screen Shot 2020-06-24 at 15 41 11

@fordarnold
Copy link
Collaborator Author

fordarnold commented Jun 30, 2020

6. Logged in as the admin, there's an excess of roles duplication. some role cards don't have edit & delete buttons

I've fixed the issue of duplicated Roles.

As for the role cards without edit/delete buttons... I was thinking that the Super Administrator role is a special type of role, and it should not be edited or deleted (i.e. if all other roles are deleted, only Super Admin should remain). Otherwise, the rest of the roles can be changed by a user with Super Administrator permissions. @bdushimi, What do you think about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Ready For Review Requires code review from Dev team TTL Review Requires code review by Technical Team Lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants