feat: (Core) Introduce Message Box component #3902
Conversation
salarenko
added
code refactoring
core
|
Deploy preview for fundamental-ngx ready! Built with commit 9ceb959 |
InnaAtanasova
changed the title
| /** @hidden */ | ||
| closeButtonClicked(): void { | ||
| this.defaultDialogConfig.closeButtonCallback(); | ||
| } | ||
|
|
||
| /** @hidden */ | ||
| approveButtonClicked(): void { | ||
| this.defaultDialogConfig.approveButtonCallback(); | ||
| } | ||
|
|
||
| /** @hidden */ | ||
| cancelButtonClicked(): void { | ||
| this.defaultDialogConfig.cancelButtonCallback(); | ||
| } | ||
| } |
There was a problem hiding this comment.
hey, didn't we discuss to add _ prefix for public but hidden props and methods?
There was a problem hiding this comment.
- Template based Message Box does not close from the Close button
- Complex template - is it intentional that the content in the Bar is not centered vertically?
- Semantic icons: I checked fund-styles and the issue is coming from there but the semantic icons are missing in the Message Box header. In styles we have additional class that removes the icon. I think you are missing this directive (or input prop in case you want to go this way)
|
@InnaAtanasova Thanks for the review, comments addressed. I've created separate issues for semantic state icons in Fundamental Styles and NGX Core #3886, #3887. I think it will be better to introduce the API when the feature is available. |
|
Object based dialog example is broken
|
| @@ -2,16 +2,14 @@ import { Component } from '@angular/core'; | |||
| import { BarElementDirective } from '../../bar/directives/bar-element.directive'; | |||
|
|
|||
| /** | |||
| * Applies fundamental layout and styling to the contents of a dialog body. | |||
| * Building block of the dialog used to create message box button. | |||
There was a problem hiding this comment.
Is this component necessary? Not adding any classes or functionality, I think the developer could just use the button component here
There was a problem hiding this comment.
Yes, this sucks. I think perfect solution would be putting buttons and stuff into the footer like:
<footer>
<fd-button></fd-button>
<fd-button></fd-button>
<footer>
and some general styling coming from the footer would add the spacing between footer direct children.
The problem is we need to use BarComponent for the implementation of the dialog header and footer. BarComponent enforces following structure:
<div fd-bar>
<div fd-bar-right>
<fd-bar-element>
<fd-button></fd-button>
</fd-bar-element>
<fd-bar-element>
<fd-button></fd-button>
</fd-bar-element>
</div>
</div>
so every bar element needs to be wrapped inside fd-bar-element which is responsible for creating spacing between bar elements.
The easiest solution is telling the user to wrap footer elements into fd-bar-element by himself. To make this more user-friendly I created fd-dialog-footer-button which is exactly the same as fd-bar-element but with more clear name. However I believe this can and should be simplified, but I wanted to tackle this separately.
Here is a separate issue for this: #3926
|
@mikerodonnell89 Object based dialog fixed. |
salarenko
force-pushed
the
fix/2430-message-box
branch
from
23e61e2 to
9ceb959
Compare
Please provide a link to the associated issue.
Closes: #2430
Issues found: #3903, #3887, #3886, #3926
Please provide a brief summary of this pull request.
This PR:
fd-dialog-titledirective in favor of universalfd-titleDefaultDialogObjecttoDialogDefaultContentDialogbehavior toDialogBaseDialogReftoDialogRefBaseDialogConfigtoDialogConfigBaseDialogFooterbehavior toDialogFooterBaseDialogServicebehavior toDialogBaseServiceDialogHeaderbehavior toDialogHeaderBaseDialogCloseButtonDirectivetoDialogCloseButtonComponentDialogService.openmethodDefaultDialogObjecttoDialogContentBaseInitialFocusModuleDynamicComponentContainerbase classDialogConfigandDialogRefto specifydatatypeBREAKING CHANGE
fd-dialog-title, fd-title should be used instead.Please check whether the PR fulfills the following requirements
https://github.com/SAP/fundamental-ngx/blob/master/CONTRIBUTING.md
https://github.com/SAP/fundamental-ngx/wiki/PR-Review-Checklist
Documentation checklist:
README.md