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

Add experimental sections view #19846

Merged
merged 51 commits into from
Feb 22, 2024
Merged

Add experimental sections view #19846

merged 51 commits into from
Feb 22, 2024

Conversation

piitaya
Copy link
Member

@piitaya piitaya commented Feb 21, 2024

Proposed change

This feature is tagged as experimental because some API or features can change. Also, there may be some bugs and optimization to be done.

CleanShot 2024-02-21 at 12 02 31

An new view type is added and allows user to use sections in dashboards views.
"Sections" view lets you to group cards within multiple section without using vertical stack card, horizontal stack card or grid card. For now, only grid section is added and lets you organize your card on section using a 4 column grid. Other type of section may arrived in the future.

A new key is added in YAML : sections. Each section can have a title and a list of cards.

Example of configuration

views:
  - type: sections
    sections:
      - type: grid
        title: Living room
        cards:
          - type: sensor
            entity: sensor.living_room_temperature
            graph: line
          - type: tile
            entity: climate.living_room
      - type: grid
        title: Bedroom
        cards:
          - type: tile
            entity: sensor.bedroom_temperature
          - type: tile
            entity: light.bed_light

Only few cards has been adapted to sections : tile, sensor and buttons. API to let the card provide its size on the grid is not finalized yet.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@piitaya piitaya marked this pull request as ready for review February 21, 2024 11:40
Copy link
Contributor

@silamon silamon left a comment

Choose a reason for hiding this comment

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

I've tested the new experimental dashboard layout and I couldn't find any bugs. Well done on the user interface.

src/panels/lovelace/editor/card-editor/hui-card-picker.ts Outdated Show resolved Hide resolved
Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

Very nice! 🎉


@property({ type: Number, attribute: "swap-threshold" })
public swapThreshold?: number;
@property({ type: String })
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep this an attribute now it can also be something else than a string?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we remove the attribute, we won't be able to do group="card" right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

src/panels/lovelace/cards/hui-stack-card.ts Outdated Show resolved Hide resolved
@@ -469,6 +481,9 @@ export class HuiTileCard extends LitElement implements LovelaceCard {
transition:
box-shadow 180ms ease-in-out,
border-color 180ms ease-in-out;
display: flex;
flex-direction: column;
justify-content: space-between;
Copy link
Member

Choose a reason for hiding this comment

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

From my POC, I needed to render hui-card-features only when this._config.features was actually set (which would save some resources anyway), is that not needed here, will it not add space between the items?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the component from the render if no features.

src/panels/lovelace/components/hui-card-edit-mode.ts Outdated Show resolved Hide resolved
Comment on lines +131 to +133
const suggestedCards = this._suggestedCards(this._cards);
const othersCards = this._otherCards(this._cards);
const customCardsItems = this._customCards(this._cards);
Copy link
Member

Choose a reason for hiding this comment

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

Why not have _loadCards do this?

Personally I would make _loadCards a memoized function passing lokalize and suggestedCards.

Then for _filterCards you just combine the 3 arrays.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will refactor that is another PR 👍

return nothing;
}

// Todo improve
Copy link
Member

Choose a reason for hiding this comment

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

Is that for a new PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I will push another PR for this one

src/panels/lovelace/strategies/get-strategy.ts Outdated Show resolved Hide resolved
Comment on lines 46 to 47
getCardSize(): number | Promise<number>;
getSize?(): [number, number];
Copy link
Member

Choose a reason for hiding this comment

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

We need to think about these names 😅 confusing... first should now be getHeight or something...

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, I will include experimental in the name so custom card doesn't use it

Copy link
Member

@balloob balloob Feb 22, 2024

Choose a reason for hiding this comment

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

let's mention in the name what the size is for. Like getGridSize.

Also, should this also be able to be a promise or not? (can always be added later too ofc)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would try to avoid having a card defining it's size async because it will shift the grid during the loading.

Copy link
Member

Choose a reason for hiding this comment

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

It will need to be asynchronous for a card that wraps another custom card, as that card can be lazy loaded

`;
}

private _addSection(): void {
Copy link
Member

Choose a reason for hiding this comment

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

I would love to move the editor code out of here in a future PR, so it will not slow down none edit usage

Copy link
Member Author

Choose a reason for hiding this comment

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

Me too! I had some issue with sortable when trying to move it out but it should be solvable!


this._sections = config.sections.map((sectionConfig, index) => {
const element = this.createSectionElement(sectionConfig);
element.index = index;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we set hass here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's already set inside createSectionElement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants