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

mgr/dashboard: Add helper component #20971

Merged
merged 1 commit into from Mar 23, 2018

Conversation

Projects
None yet
5 participants
@ricardoasmarques
Copy link
Member

ricardoasmarques commented Mar 20, 2018

This PR adds a component that can be used to provide additional information to the user.

Example:

screenshot from 2018-03-20 10-15-05

Signed-off-by: Ricardo Marques rimarques@suse.com

@LenzGr

This comment has been minimized.

Copy link
Contributor

LenzGr commented Mar 20, 2018

Nice! Would it make sense to start a section in HACKING.rst that lists all of the available components that an UI developer can make use of?

@@ -0,0 +1,8 @@
<ng-template #popoverTpl><div [innerHtml]="html"></div></ng-template>

This comment has been minimized.

@tspmelo

tspmelo Mar 20, 2018

Contributor

Can't you use ngContent here? Instead of passing the html via @Input.
That way we could show more complex html.

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Mar 21, 2018

Author Member

I've changed the implementation to support both.

1 - Useful when the helper text is stored on some controller property:

this.helperHtml = 'This works';

<cd-helper [html]="helperHtml">
</cd-helper>

2 - Useful when the helper text is declared directly in the template:

<cd-helper>
  This also works
</cd-helper>

@ricardoasmarques ricardoasmarques force-pushed the ricardoasmarques:wip-helper-component branch 2 times, most recently from eef40fb to 8c33552 Mar 21, 2018

@ricardoasmarques

This comment has been minimized.

Copy link
Member Author

ricardoasmarques commented Mar 21, 2018

@LenzGr I've added a "Frontend components" section to the HACKING.rst

@ricardoasmarques ricardoasmarques force-pushed the ricardoasmarques:wip-helper-component branch from 8c33552 to c1b0bdc Mar 21, 2018

`src/pybind/mgr/dashboard/frontend/src/app/shared/components`.

Helper
~~~~~~

This comment has been minimized.

@tspmelo

tspmelo Mar 21, 2018

Contributor

This should be a sub-item of "Frontend components"

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Mar 22, 2018

Author Member

Fixed

@ricardoasmarques ricardoasmarques force-pushed the ricardoasmarques:wip-helper-component branch from c1b0bdc to 2126a52 Mar 22, 2018

[popover]="popoverTpl"
placement="bottom"
container="body"
[outsideClick]="true">

This comment has been minimized.

@votdev

votdev Mar 23, 2018

Contributor

Is property binding really necessary here?

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Mar 23, 2018

Author Member

Fixed

This comment has been minimized.

@tspmelo

tspmelo Mar 23, 2018

Contributor

This should be a property binding.

Otherwise angular will not interpret the value and will see it as a string and not a boolean.
p.e.: If you set outsideClick="false" popover component will think the value is actually true, because it is a string and not empty.

Only situations when we should remove the binding is if the value is actually a string and it will never change (Or you know the component will convert the string to the correct type).

This comment has been minimized.

@ricardoasmarques

ricardoasmarques Mar 23, 2018

Author Member

@tspmelo I've tested it and you are right.Thanks.

@votdev I've reverted the change, so I'm using property binding again.

@ricardoasmarques ricardoasmarques force-pushed the ricardoasmarques:wip-helper-component branch from 2126a52 to 93ea821 Mar 23, 2018

@votdev

votdev approved these changes Mar 23, 2018

[popover]="popoverTpl"
placement="bottom"
container="body"
outsideClick="true">

This comment has been minimized.

@tspmelo

tspmelo Mar 23, 2018

Contributor

This should be a property binding.

mgr/dashboard: Add helper component
Signed-off-by: Ricardo Marques <rimarques@suse.com>

@ricardoasmarques ricardoasmarques force-pushed the ricardoasmarques:wip-helper-component branch from 93ea821 to 94c5a5e Mar 23, 2018

@rjfd rjfd merged commit 7a490ac into ceph:master Mar 23, 2018

4 of 5 checks passed

make check (arm64) make check failed
Details
Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment