[UI Framework] KuiGalleryItem automatically becomes link or button #14240
Conversation
timroes
added
Team:Platform-Design
| @@ -2,18 +2,41 @@ import React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import classNames from 'classnames'; | |||
|
|
|||
| export const KuiGalleryItem = ({ children, className, ...rest }) => { | |||
| const checkHrefAndOnClick = (props, propName, componentName) => { | |||
There was a problem hiding this comment.
Nice. I should add this to K7 stuff too. I wasn't this detailed to check for both.
| const component = <KuiGalleryItem href="#" {...requiredProps}>children</KuiGalleryItem>; | ||
| expect(render(component)).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| test('renders KuiGalleryItem with onClick', () => { | ||
| const component = <KuiGalleryItem onClick={() => {}} {...requiredProps}>children</KuiGalleryItem>; |
There was a problem hiding this comment.
Instead of doing a snapshot comparison, we should test that the callback is called when the element is clicked, similar to how our KuiButton tests work:
describe('onClick', () => {
test(`isn't called upon instantiation`, () => {
const onClickHandler = sinon.stub();
shallow(
<KuiButton onClick={onClickHandler} />
);
sinon.assert.notCalled(onClickHandler);
});
test('is called when the button is clicked', () => {
const onClickHandler = sinon.stub();
const $button = shallow(
<KuiButton onClick={onClickHandler} />
);
$button.simulate('click');
sinon.assert.calledOnce(onClickHandler);
});
});Could you also add an example of a KuiGalleryItem using onClick in the doc site and verify that it gets applied and called when you click?
|
@cjcenizal could you give it another look? |
There was a problem hiding this comment.
@timroes Thanks for the ping, sorry about the delay! Could you also add an example of a KuiGalleryItem using onClick in the doc site and verify that it gets applied and called when you click, e.g. by making the handler call window.alert? After that this LGTM!
|
Thanks for the feedback. Could you give it one last look with the doc, then I would merge it :-) |
…14240) * KuiGalleryItem automatically becomes link or button * Add onClick tests * Add doc example with onClick
|
Backports:
|
…lastic#14240) * KuiGalleryItem automatically becomes link or button * Add onClick tests * Add doc example with onClick
…lastic#14240) * KuiGalleryItem automatically becomes link or button * Add onClick tests * Add doc example with onClick
As discussed with @cjcenizal introducing with this PR the
KuiGalleryItemwill now automatically decide whether it needs to be anaor abuttonelement.If an
hrefproperty is specified it will become anaelement, otherwise it will become abutton. An prop type error is thrown when both properties are specified.This is mainly done for accessibility reasons, so that keyboard accessibility will work properly in all circumstances.