-
Notifications
You must be signed in to change notification settings - Fork 18
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
UIIN-2452: Enable/disable consortial holdings/item actions based on User permissions #2284
Changes from 33 commits
e9ff698
f5c029c
4a08c0c
5dfcf0d
188acc3
58f4cd4
dc2e0d0
108f852
3f42476
9e1b138
872f7be
c8e3f29
f57ce13
5726f06
d25021e
767a4a1
5769fca
fc4b05a
1c5bcd1
081f18b
60b4011
9c397c8
483d77e
7894483
a7efac1
42f16f0
22d2dc9
ae50bd4
69396b4
4244578
39c34b5
27fc561
2398063
a603d03
b313553
e18e43c
f72699c
10a2e3a
773e3c1
76a05f7
4d4d213
355c482
00589ea
51f91bf
e81a420
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ import { | |
Icon, | ||
} from '@folio/stripes/components'; | ||
|
||
import { switchAffiliation } from '../../../utils'; | ||
|
||
import { MoveToDropdown } from './MoveToDropdown'; | ||
|
||
const HoldingButtonsGroup = ({ | ||
|
@@ -24,6 +26,9 @@ const HoldingButtonsGroup = ({ | |
onAddItem, | ||
itemCount, | ||
isOpen, | ||
tenantId, | ||
isViewHoldingsDisabled, | ||
isAddItemDisabled, | ||
}) => { | ||
const stripes = useStripes(); | ||
const isUserInCentralTenant = checkIfUserInCentralTenant(stripes); | ||
|
@@ -42,7 +47,8 @@ const HoldingButtonsGroup = ({ | |
<Button | ||
id={`clickable-view-holdings-${holding.id}`} | ||
data-test-view-holdings | ||
onClick={onViewHolding} | ||
onClick={() => switchAffiliation(stripes.okapi, tenantId, onViewHolding)} | ||
disabled={isViewHoldingsDisabled} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we shouldn't display elements if a user doesn't have permission. I guess it's a FOLIO pattern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yep, that's the FOLIO UX I'm familiar with. I will follow up with the PO here as I see the story was written specifically with disabled elements, and I've seen similar stories in ui-users. We need to figure out if we are purposely abandoning that pattern or if this was just a goof. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zburke and @Dmytro-Melnyshyn This is a requirement for this story, but we discussed with PO that suppressing these buttons from a user instead of disabling them will make the page kinda less noisy and will free up more space. We decided to go with the disabled buttons in the main release and after further testing and feedback from users, we might end up hiding these buttons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I understand the story was written this way, but the point @Dmytro-Melnyshyn and I are making is that the story was written incorrectly. Given that we know the story was written incorrectly, why would deliberately do the wrong thing and add debt when we could just do it the right thing? |
||
> | ||
<FormattedMessage id="ui-inventory.viewHoldings" /> | ||
</Button> | ||
|
@@ -52,8 +58,9 @@ const HoldingButtonsGroup = ({ | |
<Button | ||
id={`clickable-new-item-${holding.id}`} | ||
data-test-add-item | ||
onClick={onAddItem} | ||
onClick={() => switchAffiliation(stripes.okapi, tenantId, onAddItem)} | ||
buttonStyle="primary paneHeaderNewButton" | ||
disabled={isAddItemDisabled} | ||
> | ||
<FormattedMessage id="ui-inventory.addItem" /> | ||
</Button> | ||
|
@@ -73,6 +80,9 @@ HoldingButtonsGroup.propTypes = { | |
onAddItem: PropTypes.func.isRequired, | ||
onViewHolding: PropTypes.func.isRequired, | ||
withMoveDropdown: PropTypes.bool, | ||
isViewHoldingsDisabled: PropTypes.bool, | ||
isAddItemDisabled: PropTypes.bool, | ||
tenantId: PropTypes.string, | ||
}; | ||
|
||
|
||
|
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.
Why do you use
window.location.href
instead ofreact-router
API?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.
@BogdanDenis To refresh the page, see the comment from Dmytro please, I described the process there