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.
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
#35715 workspace member details page #37715
#35715 workspace member details page #37715
Changes from 16 commits
35f5618
be52759
69ee8a1
210d294
8e0f762
6dc39ce
9409582
ec5458b
d50d4a4
d916cae
8000440
e29e0b2
b37abfe
f583fd4
f24deca
27f7917
7a91ec8
6775986
07280c7
aa11bf3
de3c302
a85b5e7
dc7320e
86bf243
33509bb
1159171
f2b95c7
f78a65e
dcd13bb
0140c46
18d88ef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
We are not checking if user is workspace member or not, ref: #41492
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.
I think we need to block these views if the user is not an admin. We can reuse
AdminPolicyAccessOrNotFoundWrapper
andPaidPolicyAccessOrNotFoundWrapper
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.
Added, but are you sure this feature should be available only for Paid Workspaces? I must have missed this requirement...
Should I also disable showing RHN when pressing on the member item if its non Paid Workspace?
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.
@luacmartins ok, I think disabling RHN for non Paid Workspace is most reasonable - I've made this change, so now:
Please see the recording:
Screen.Recording.2024-03-07.at.14.30.22.mov
What do you think?
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.
Personally I don't like that there would be variability in pressing the whole item triggering the select action or not. I think we should limit selecting a row to just engaging with the checkbox, no matter what the case is. Curious if @Expensify/design agrees here.
So basically, let's not try to get tricky with using the whole row as a trigger action for the select box. Just make it so that you can only ever use the select box to select a row. For non-paid workspaces, just make the rest of the row non-tappable.
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.
@shawnborton makes sense to me - I'll change it
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.
done
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.
Totally agree @shawnborton.
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.
I don't see the tick mark for the very first time i open a member with the new changes
Screen.Recording.2024-03-06.at.5.26.50.PM.mov
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.
is this a member that was invited right before? there is known issue that we need to address regarding this... the same problem is with bulk actions and "Make admin" option for freshly invited members (before refreshing the page) - I think there was some bug for it
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 is related comment: #37748 (comment) - we agreed to fix this while working on inviting new members here: #37199 (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.
Got it.