-
Notifications
You must be signed in to change notification settings - Fork 270
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
feat(ui5-card): fires headerPress event upon header click #250
Changes from 6 commits
d4a5c68
6caf75d
445b0a7
c45794b
83a07d4
a39f9ac
6d13152
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 |
---|---|---|
|
@@ -3,6 +3,8 @@ import URI from "@ui5/webcomponents-base/src/types/URI"; | |
import Bootstrap from "@ui5/webcomponents-base/src/Bootstrap"; | ||
import ShadowDOM from "@ui5/webcomponents-base/src/compatibility/ShadowDOM"; | ||
import { isIconURI } from "@ui5/webcomponents-base/src/IconPool"; | ||
import { isSpace, isEnter } from "@ui5/webcomponents-base/src/events/PseudoEvents"; | ||
import Function from "@ui5/webcomponents-base/src/types/Function"; | ||
import CardRenderer from "./build/compiled/CardRenderer.lit"; | ||
import Icon from "./Icon"; | ||
|
||
|
@@ -82,6 +84,26 @@ const metadata = { | |
type: URI, | ||
defaultValue: null, | ||
}, | ||
|
||
_headerActive: { | ||
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. activeHeader? 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. This one is for the pressed state, but there could be a public property determining wether the header is active or not (activeHeader as you said) and then this would be renamed to _pressedHeader or similar. |
||
type: Boolean, | ||
}, | ||
|
||
_headerPress: { | ||
type: Function, | ||
}, | ||
}, | ||
events: /** @lends sap.ui.webcomponents.main.Card.prototype */ { | ||
|
||
/** | ||
* Fired when the <code>ui5-card</code> header is pressed | ||
* by click/tap or by using the Enter or Space key. | ||
* | ||
* @event | ||
* @public | ||
* @since 0.10.0 | ||
*/ | ||
headerPress: {}, | ||
}, | ||
}; | ||
|
||
|
@@ -107,6 +129,12 @@ const metadata = { | |
* @public | ||
*/ | ||
class Card extends WebComponent { | ||
constructor() { | ||
super(); | ||
|
||
this._headerPress = this.headerPress.bind(this); | ||
} | ||
|
||
static get metadata() { | ||
return metadata; | ||
} | ||
|
@@ -125,6 +153,12 @@ class Card extends WebComponent { | |
image, | ||
ctr: state, | ||
renderIcon: state.icon && !state.image, | ||
classes: { | ||
header: { | ||
"sapFCardHeader": true, | ||
"sapFCardHeaderActive": state._headerActive, | ||
}, | ||
}, | ||
}; | ||
} | ||
|
||
|
@@ -133,6 +167,42 @@ class Card extends WebComponent { | |
|
||
super.define(...params); | ||
} | ||
|
||
headerPress(event) { | ||
const click = event.type === "click"; | ||
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 am not really sure how stable is this. 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, this approach is arguable, we can discuss it briefly today. |
||
const keydown = event.type === "keydown"; | ||
|
||
if (click) { | ||
this.fireEvent("headerPress"); | ||
return; | ||
} | ||
|
||
if (isEnter(event)) { | ||
this._handleEnter(keydown); | ||
return; | ||
} | ||
|
||
if (isSpace(event)) { | ||
event.preventDefault(); | ||
this._handleSpace(keydown); | ||
} | ||
} | ||
|
||
_handleEnter(keydown) { | ||
if (keydown) { | ||
this.fireEvent("headerPress"); | ||
} | ||
|
||
this._headerActive = keydown; | ||
} | ||
|
||
_handleSpace(keydown) { | ||
if (!keydown) { | ||
this.fireEvent("headerPress"); | ||
} | ||
|
||
this._headerActive = keydown; | ||
} | ||
} | ||
|
||
Bootstrap.boot().then(_ => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,15 @@ | ||
const assert = require('assert'); | ||
const assert = require('assert'); | ||
describe("Card general interaction", () => { | ||
browser.url("http://localhost:8080/test-resources/sap/ui/webcomponents/main/pages/Card.html"); | ||
|
||
it("fires headerPress upon click, Enter and Space", () => { | ||
const cardHeader = browser.findElementDeep("#card >>> .sapFCardHeader"); | ||
const field = browser.$("#field"); | ||
|
||
cardHeader.click(); | ||
cardHeader.keys("Space"); | ||
cardHeader.keys("Enter"); | ||
|
||
assert.strictEqual(field.getProperty("value"), "3", "headerPress should be called 3 times"); | ||
}); | ||
}); |
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.
there is a native type function so I think we can skip this?
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.
There is, but the metadata validation would not pass for now. As this refactoring is started by Vladi, I will change it to native Function once it's done.