Permission groups section #406
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dominik-zeglen
suggested changes
Feb 28, 2020
There was a problem hiding this comment.
A few things:
- PG section is not visible in navigator
- These buttons are not aligned properly
- Bulk actions do not work
- react throws error when checkbox is clicked
- missing mutation
- Table cells width should be fixed, because selecting users cause cells to expand
- Selecting users should apply a proper style for the table row
- The text should be centered and padding from both top and bottom should be increased.
- Since we enabled support for
?.operator, new code should not usemaybeso we slowly move away from this pattern. - Sorting users in group details view doesn't work
src/permissionGroups/components/PermissionGroupCreatePage/PermissionGroupCreatePage.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupCreate/PermissionGroupCreate.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupDetails/PermissionGroupDetails.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupDetails/PermissionGroupDetails.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupDetails/PermissionGroupDetails.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupList/PermissionGroupList.tsx
Outdated
Show resolved
Hide resolved
krzysztofwolski
force-pushed
the
permission-groups
branch
from
3b1d208
to
adf0035
Compare
Codecov Report
@@ Coverage Diff @@
## master #406 +/- ##
==========================================
- Coverage 72.39% 72.19% -0.20%
==========================================
Files 658 683 +25
Lines 7441 7705 +264
Branches 1227 1290 +63
==========================================
+ Hits 5387 5563 +176
- Misses 1819 1886 +67
- Partials 235 256 +21
Continue to review full report at Codecov.
|
|
Blocked by #453 |
krzysztofwolski
force-pushed
the
permission-groups
branch
from
0944127
to
c48bd55
Compare
krzysztofwolski
force-pushed
the
permission-groups
branch
2 times, most recently
from
3506484
to
adb3940
Compare
dominik-zeglen
suggested changes
Apr 15, 2020
src/components/AccountPermissionGroups/AccountPermissionGroups.tsx
Outdated
Show resolved
Hide resolved
src/components/AccountPermissionGroups/AccountPermissionGroups.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/components/AssignMembersDialog/AssignMembersDialog.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/components/AssignMembersDialog/AssignMembersDialog.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/components/AssignMembersDialog/AssignMembersDialog.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/components/PermissionGroupMemberList/PermissionGroupMemberList.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/components/PermissionGroupMemberList/PermissionGroupMemberList.tsx
Outdated
Show resolved
Hide resolved
src/permissionGroups/views/PermissionGroupDetails/PermissionGroupDetails.tsx
Outdated
Show resolved
Hide resolved
| @@ -20,6 +20,9 @@ const messages = defineMessages({ | |||
| }, | |||
| tooSimilar: { | |||
| defaultMessage: "These passwords are too similar" | |||
| }, | |||
| unique: { | |||
| defaultMessage: "This needs to be unique" | |||
There was a problem hiding this comment.
I'm not sure if this error should have generic error message 🤷♂️
There was a problem hiding this comment.
Do you think it should be "Name should be unique"?
There was a problem hiding this comment.
Depends what should be unique, "name" and "slug" could have different messages. It would be nice to know what field(s) could throw this error and make proper function for each field.
There was a problem hiding this comment.
It could be thrown only by name field so I don't think separate functions are needed. Change to 'Name needs to be unique'?
krzysztofwolski
force-pushed
the
permission-groups
branch
2 times, most recently
from
d98d5dd
to
37a1f7f
Compare
|
Note: something weird happened to schema, prettier maybe? |
dominik-zeglen
approved these changes
Apr 22, 2020
src/permissionGroups/components/MembersErrorDialog/MembersErrorDialog.tsx
Outdated
Show resolved
Hide resolved
krzysztofwolski
force-pushed
the
permission-groups
branch
from
508da57
to
fa4d8f2
Compare
krzysztofwolski
requested review from
dominik-zeglen
and removed request for
dominik-zeglen
dominik-zeglen
approved these changes
Apr 23, 2020
dominik-zeglen
approved these changes
Apr 23, 2020
This was referenced Mar 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I want to merge this change because...
adds frontend for saleor/saleor#5176
To be used with API branch: feature/permission-groups
Screenshots
Pull Request Checklist
.potfile.