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

create spacer chip #1098

Merged
merged 7 commits into from
Apr 17, 2023
Merged

create spacer chip #1098

merged 7 commits into from
Apr 17, 2023

Conversation

bryan-stewart
Copy link
Contributor

Description

IMG_5433

I added a spacer chip to the chips card. Just a div element with flex-grow. currently no configuration options but setting custom sizes could be added.

Related Issue

This PR fixes or closes issue: fixes #1096

Motivation and Context

Seems like a good idea to have spacers in the chips card

How Has This Been Tested

Tested in Firefox, Chrome, Edge, Companion App(iOS)

Types of changes

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 🌎 Translation (addition or update a translation)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have tested the change locally.

Comment on lines 36 to 40
if (!this.hass || !this._config) {
return html`<div></div>`;
}

return html`<div></div>`;
Copy link

@RickeyWard RickeyWard Apr 6, 2023

Choose a reason for hiding this comment

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

This code doesn't seem to have any effect, would probably drop it.

Suggested change
if (!this.hass || !this._config) {
return html`<div></div>`;
}
return html`<div></div>`;
return html`<div></div>`;

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, hass and config are not used, we don't need these attributes in the component.

Copy link
Owner

@piitaya piitaya left a comment

Choose a reason for hiding this comment

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

Hello. I like the idea. It can be very useful.
Thanks's for your contribution. The code need to be cleaned because there is many unused code in the editor and the chip.
Also, the documentation is missing.

Comment on lines 41 to 47
<ha-form
.hass=${this.hass}
.data=${this._config}
.schema=${schema}
.computeLabel=${this._computeLabel}
@value-changed=${this._valueChanged}
></ha-form>
Copy link
Owner

Choose a reason for hiding this comment

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

You can delete this because it's not needed.

Comment on lines 24 to 31
private _computeLabel = (schema: HaFormSchema) => {
const customLocalize = setupCustomlocalize(this.hass!);

if (GENERIC_LABELS.includes(schema.name)) {
return customLocalize(`editor.card.generic.${schema.name}`);
}
return this.hass!.localize(`ui.panel.lovelace.editor.card.generic.${schema.name}`);
};
Copy link
Owner

Choose a reason for hiding this comment

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

You can delete this because it's not needed.

import { SpacerChipConfig } from "../../../utils/lovelace/chip/types";
import { LovelaceChipEditor } from "../../../utils/lovelace/types";

const computeSchema = memoizeOne((): HaFormSchema[] => []);
Copy link
Owner

Choose a reason for hiding this comment

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

You can delete this because it's not needed.

Comment on lines 48 to 49
<div style="margin-top: 20px;">There are currently no config options for this chip.</div>
`;
Copy link
Owner

Choose a reason for hiding this comment

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

This should be translated.

Comment on lines 36 to 40
if (!this.hass || !this._config) {
return html`<div></div>`;
}

return html`<div></div>`;
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, hass and config are not used, we don't need these attributes in the component.

@bryan-stewart
Copy link
Contributor Author

Sorry, I left the config and form stuff in because I wasn't sure if some config options would be wanted.

I removed the unused code except the setConfig function as it is required on interface LovelaceChip. I'm not sure what is best to do with that.

I added docs and an English translation

@bryan-stewart
Copy link
Contributor Author

If I prevent the edit button from rendering for the spacer chip in chips-card-chips-editor then I can get rid of the editor all together. Removes the need for the translation.

Screenshot 2023-04-06 142340

@piitaya
Copy link
Owner

piitaya commented Apr 7, 2023

Yes, we can remove the editor and the edit button 😊

@piitaya piitaya merged commit 4d5f67f into piitaya:main Apr 17, 2023
@piitaya piitaya added the enhancement New feature or request label Apr 17, 2023
@alexvaltchev
Copy link

I updated to the last version but I dont see SPACER available? Why? v2.7.0
Screenshot_2

@aviadlevy
Copy link

Make sure you refresh your browser. working well for me
image

@schmuy
Copy link

schmuy commented Apr 19, 2023

I updated to the last version but I dont see SPACER available? Why? v2.7.0

Screenshot_2

Same issues. I don't see spacer available in safari or iOS companion app. It does show in chrome and I can add it in chrome and it behaves correctly. When going back to chrome or iOS it does not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Chip Spacer
6 participants