Skip to content

Consider default_view as view by default.#6

Closed
andrey-git wants to merge 2 commits into
home-assistant:masterfrom
andrey-git:patch-1
Closed

Consider default_view as view by default.#6
andrey-git wants to merge 2 commits into
home-assistant:masterfrom
andrey-git:patch-1

Conversation

@andrey-git
Copy link
Copy Markdown
Contributor

Consider default_view as view by default.

Will allow to use home-assistant/frontend#244 without manually specifying view: true under default_view

@mention-bot
Copy link
Copy Markdown

@andrey-git, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob to be a potential reviewer.

Comment thread lib/view.js Outdated
// Entity is a view if it has the 'view' attribute set.
// Consider default_view as view by default.
if (entity.attributes.view ||
(entity.entity_id === DEFAULT_VIEW_ENTITY_ID && entity.attributes.view !== false)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You test entity.attributes.view !== false, which is equivalent to testing entity.attributes.view which is what we do in the first part of this logical expression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, actually it either true or undefined, no way for it to be false (it just won't be set in group.py)

I removed the extra check. Now default_view is always considered a view.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2017

As far as I know, the default view is specified via the groups command and thus will always have view: true

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2017

If people don't set view: true in their config, it's a configuration problem. We should not try to fix this here because that means that we will have to fix it in every UI. Instead, the user has to configure it correctly.

@andrey-git
Copy link
Copy Markdown
Contributor Author

The current UI works fine without view: true on default_view.

This function will consider it non-view, but partial-cards.html check by entity id.

Should I add a voluptuous check instead that default_view must have view: true?

I think it is extra-verbose :)

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2017

Yes, a voluptuous check is the right place. We should never fight symptoms but instead solve things at the source.

@andrey-git
Copy link
Copy Markdown
Contributor Author

Created home-assistant/core#6758 instead.

@andrey-git andrey-git deleted the patch-1 branch March 23, 2017 17:09
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