-
Notifications
You must be signed in to change notification settings - Fork 133
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
fix: button fiori3 fixes and documentation improvements #2221
Conversation
bc39583
to
ff50df6
Compare
@@ -18,7 +18,7 @@ | |||
<separator></separator> | |||
|
|||
<fd-docs-section-title [id]="'Button-sizes'" [componentName]="'button'"> Button Sizes </fd-docs-section-title> | |||
<description> Use <code class="code-snippet">[compact]="true"</code> for the mobile button size. </description> | |||
<description> Use (or skip) <code class="code-snippet">[compact]="false"</code> for the mobile button size. </description> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<button class="programmable-button" fd-button (click)="isOpen=true">Open</button> | ||
<button class="programmable-button" fd-button (click)="isOpen=false">Close</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have mixed feelings about this :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the class to docs-button
| 'positive' | ||
| 'negative' | ||
| 'attention' | ||
| 'half' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we keeping the half button? @droshev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to keep something that is not part of Fiori 3. At least not in the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed half
from docs but for now I didn't remove half
from component
@@ -41,50 +52,58 @@ export class ButtonComponent implements OnInit, CssClassBuilder { | |||
this._change(); | |||
} // user's custom classes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment should be TypeDocs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes applied
@@ -23,5 +23,5 @@ export class SegmentedButtonComponent { | |||
|
|||
/** @hidden */ | |||
@HostBinding('class.fd-segmented-button') | |||
fdButtonGroupClass: boolean = true; | |||
fdsegmentedButtonClass: boolean = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe fdSegmentedButtonClass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats the reason for using fd
prefix for variable names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable name changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments but they are minor so feel free to merge it once @droshev approves. It's good to have everything merged to feat/button-dialog-fonts
so we test it tomorrow
* fix: few fixes in the code and documentation found after tests * small fixes * workaround added to fix missing selector * fix: addresing pr comments
* fix: few fixes in the code and documentation found after tests * small fixes * workaround added to fix missing selector * fix: addresing pr comments
…2224) * feat: addopt styles for buttons to match fiori 3 * fix: bring icons and fonts (#2187) * fix: Migrate modal to dialog support Fiori3 design and features (#2189) * fix(core) rename modal to dialog * Reworking dialogs header * Refactor dialogs footer * fix(core) Add draggable dialog support * fix(core) Improve dynamic component and dialog service code * fix(core) Introduce custom injector to DynamicComponents | Inject configs to Alert and Notification * fix(core) Create new Dialog Container | Use InjectionTokens with parent assecuration * fix(core) Create resize directive * Update Dialog documentation * fix(core) Add responsive paddings feature to dialog * fix(core) Add CssClassBuilders to Dialog and DialogContainer * fix(core) Update descriptions * fIX(Core) Revert Alert and Notification changes * fix(core) Add unit tests * fix(core) update missing buttons adnotations * fix(core) Update popover example * fix(core) Fix indirect imports * fix(core) Add loading state do Dialog * fix(core) Update tests * fix(core) Update tests * fix(core) Fix alert and notifications * fix(core) Address PR requests * fix(core) Fix resize for IE11 * fix: button fiori3 fixes and documentation improvements (#2221) * fix: few fixes in the code and documentation found after tests * small fixes * workaround added to fix missing selector * fix: addresing pr comments * fix: bring icons to angular.json file (#2227) * changes to support new code contribution guidline * fix(core) Add footer buttons wrapper * fix(core) Apply documentation changes according to PR comments * added ngoninit to button * fix(core) Create 'closestElement' function * Update button-docs.component.html * Update date-picker-complex-i18n-example.component.html * Update dialog-docs-header.component.html * Update dialog-docs.component.html * Update component-based-dialog-example.component.ts * Update dialog-backdrop-container-example.component.ts * fix closestElement tests * chore: bump styles and theming package versions * fix: remove dialog styling * fix: shellbar product menu markup update * menu button fixed * fix(core) fix Dialog styles | Change dialog buttons order * Add NODE_OPTIONS to netlify.toml * feat: fd-button--transparent added to input-group component * fix shellbar styling issues * chore: bump fundamental-styles version * fix(core) Clear selected products in complex dialog example Co-authored-by: Gracjan Górecki <gorecki.gracjan@hotmail.com> Co-authored-by: Inna Atanasova <39598672+InnaAtanasova@users.noreply.github.com> Co-authored-by: Vanessa-Cusmich <54723167+Vanessa-Cusmich@users.noreply.github.com> Co-authored-by: deno <mladen.droshev@sap.com> Co-authored-by: Mike O'Donnell <mikerodonnell89@users.noreply.github.com>
Please provide a link to the associated issue.
Please provide a brief summary of this pull request.
The PR brings:
deprecated
label in button component for properties being deprecated (for documentation purpose)Please check whether the PR fulfills the following requirements
https://github.com/SAP/fundamental-ngx/blob/master/CONTRIBUTING.md
Documentation checklist:
Screenshots
Before
Segmented button API not built
Options not marked as deprecated
The default value for params are missing
RTL mode not working
After
Segmented button API not built
Options not marked as deprecated
The default value for params are missing
RTL mode not working