Skip to content
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: add device type's icons to cards #1350

Merged

Conversation

tbrkollar
Copy link
Collaborator

No description provided.

@tbrkollar tbrkollar requested a review from brdandu June 28, 2024 12:36
@tbrkollar tbrkollar self-assigned this Jun 28, 2024
let category = ''
this.$store.state.zap.selectedZapConfig.zclProperties.forEach((item) => {
if (item.id === packageRef) {
category = item.category
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return category once it has been populated.

src/util/common-mixin.js Outdated Show resolved Hide resolved
src/util/common-mixin.js Outdated Show resolved Hide resolved
@@ -289,6 +300,15 @@ export default {
}
},
methods: {
getDeviceCategory(packageRef) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comments to function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the comment format as below for this function too
/** * Specifies the path to the logo image * * @param {*} isTiny - * if value "true", it's used as a tiny logo * if value "false", it's used as a normal logo * @param {*} category - * if value "zigbee", it's used as a zigbee logo * if value "matter", it's used as a matter logo * if value "multiprotocol", it's used as a multiprotocol logo * @param {*} isSelected - * if value "true", it's used as a dark / selected version * if value "false", it's used as a light version * @returns Returns a string value for the src image property */

src/util/common-mixin.js Show resolved Hide resolved
@tbrkollar tbrkollar force-pushed the feature/add-device-type-to-card branch from 981b27f to 0394873 Compare July 1, 2024 14:31
@tbrkollar tbrkollar requested a review from brdandu July 2, 2024 13:09
@@ -289,6 +300,15 @@ export default {
}
},
methods: {
getDeviceCategory(packageRef) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the comment format as below for this function too
/** * Specifies the path to the logo image * * @param {*} isTiny - * if value "true", it's used as a tiny logo * if value "false", it's used as a normal logo * @param {*} category - * if value "zigbee", it's used as a zigbee logo * if value "matter", it's used as a matter logo * if value "multiprotocol", it's used as a multiprotocol logo * @param {*} isSelected - * if value "true", it's used as a dark / selected version * if value "false", it's used as a light version * @returns Returns a string value for the src image property */

if (item.id === packageRef) {
if (item.category) {
category = item.category
return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized the returning in a forEach does not really end the loop but it continues going through the loop regardless.

Can you switch this.$store.state.zap.selectedZapConfig.zclProperties.forEach((item) => { if (item.id === packageRef) { if (item.category) { category = item.category return } } })

to

let zclProperty = this.$store.state.zap.selectedZapConfig.zclProperties.find((item) => (item.id === packageRef && item.category)) if (zclProperty) { category = zclProperty.category } return category

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also switch to a normal for loop and return instead of using forEach

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, thanks. i will fix it

@tbrkollar tbrkollar merged commit e76f3ca into project-chip:master Jul 3, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants