jwargo
commented
5 years ago @mhartington created this codepen to help me troubleshoot this: https://codepen.io/mhartington/pen/xejmOL?editors=1000
Closed jwargo closed 5 years ago
jwargo
commented
5 years ago @mhartington created this codepen to help me troubleshoot this: https://codepen.io/mhartington/pen/xejmOL?editors=1000
johnwargo
commented
5 years ago I played around with this today and I just noticed that the class gets set correctly if I open the page again. So it's only incorrect when I add an item to the list.
brandyscarney
commented
5 years ago Thanks for the issue! Could you update the code or provide an example of it broken? The code in the issue & the Codepen Mike created show the buttons looking correct.
Thanks!
jwargo
commented
5 years ago @brandyscarney sorry, I don't. I pasted in my code above when I encountered the issue Mike asked me to make the issue. I've since moved on to a different approach (which works quite well). Mike has access to the repo although I've deleted the branch.
mhartington
commented
5 years ago Example code is
<ion-item *ngFor="let button of project.buttons; let idx= index" no-padding>
<ion-button *ngIf="idx>0" (click)="buttonUp(idx)" slot="start">
<ion-icon name="arrow-dropup" slot="icon-only"></ion-icon>
</ion-button>
<ion-label text-wrap (click)="buttonModify(button)"> {{button.title}} </ion-label>
<ion-button *ngIf="idx<project.buttons?.length-1" (click)="buttonDown(idx)" slot="end">
<ion-icon name="arrow-dropdown" slot="icon-only"></ion-icon>
</ion-button>
</ion-item>TL;DR, ngIf on the ion-button will prevent the size="small" attribute being set. Looks like the host data isn't being set and the css/size is not being applied.
brandyscarney
commented
5 years ago Thanks Mike! This is likely the problem code in item:
componentDidLoad() {
// Change the button size to small for each ion-button in the item
// unless the size is explicitly set
Array.from(this.el.querySelectorAll('ion-button')).forEach(button => {
if (button.size === undefined) {
button.size = 'small';
}
});We should move this to check on the button if it is in an item, or possibly move it to CSS.
ionitron-bot[bot]
commented
5 years ago This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.
If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.
For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.
Thank you!
topalavlad
commented
5 years ago I'd like to take a look at this issue but I'm a bit confused about how to write a test containing *ngFor since all other tests are very basic. I tried accessing fields declared in <script> from the HTML file but although methods are working, fields aren't. I'm guessing they should be wrapped in a class somehow.
Is there any recommended way to go about debugging this issue?
brandyscarney
commented
5 years ago Thanks @topalavlad! Our tests are written in vanilla JavaScript, so basically you wouldn't use Angular to reproduce it in an e2e test.
Since the code is happening after the item loads, I think the *ngIf on the button may be causing the issue. I created the following Codepen to reproduce this, you have to click "Add a Button" to see: https://codepen.io/brandyscarney/pen/VOMaXw?editors=1111
topalavlad
commented
5 years ago We should move this to check on the button if it is in an item, or possibly move it to CSS.
I tried doing it via css but unfortunately didn't have much luck. The descendant selector doesn't seem to work. Just for testing, I removed ion-item from the selector below and all ion-button components became small so the descendant selector seems to be the problem:
:host(ion-item ion-button:not(.button-small):not(.button-default):not(.button-large)) {
@extend .button-small;
}Any suggestions?
brandyscarney
commented
5 years ago @topalavlad It might not be possible in just CSS at the moment. I was thinking of using hostContext in order to set the size on the button, like the following line:
'in-item': hostContext('ion-item', el)https://github.com/ionic-team/ionic/blob/master/core/src/components/checkbox/checkbox.tsx#L141
The problem is the size would need to update if the user sets it on the button. By default it would be small if inside of an item, unless the user set it.
This could probably just be done in the componentWillLoad of button.tsx instead like the check for in a toolbar:
componentWillLoad() {
this.inToolbar = !!this.el.closest('ion-buttons');
this.inItem = !!this.el.closest('ion-item');
}Then in hostData (this is untested code):
let size = this.size;
if (size === undefined && this.inItem) {
size = 'small';
}Is doing it in hostData better/cleaner than directly in componentWillLoad like in the PR?
brandyscarney
commented
5 years ago @topalavlad So the reason hostData is better is because it will work with the size dynamically changing. In componentWillLoad it will only set the size when the button initializes, but every time you change the size it will check for item. So if you start with the following:
<ion-item>
<ion-button slot="start" size="large">Button</ion-button>
</ion-item>And then remove the size, it will still update to use small. Your PR looks great though! I pushed a small change for this.
brandyscarney
commented
5 years ago Thanks for the issue and @topalavlad for the PR! I accidentally added an extra number sign in the merge, but this is fixed by https://github.com/ionic-team/ionic/commit/a3e23fc9fabb57bd4e09a66841f89f1bb095019b. :grin:
ionitron-bot[bot]
commented
5 years ago Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.
Bug Report
Ionic version:
[x] 4.x
Current behavior:
this code:
generates the following output. The buttons don't size the same.
Expected behavior:
The buttons would be the same size.
Steps to reproduce:
From my conversation on Slack with Mike Hartington:
Related code:
Other information:
Ionic info:
Ionic:
ionic (Ionic CLI) : 4.12.0 (/Users/johnwargo/.nvm/versions/node/v11.12.0/lib/node_modules/ionic) Ionic Framework : @ionic/angular 4.0.1 @angular-devkit/build-angular : 0.12.4 @angular-devkit/schematics : 7.2.4 @angular/cli : 7.2.4 @ionic/angular-toolkit : 1.3.0
Capacitor:
capacitor (Capacitor CLI) : 1.0.0-beta.19 @capacitor/core : 1.0.0-beta.17
System:
NodeJS : v11.12.0 (/Users/johnwargo/.nvm/versions/node/v11.12.0/bin/node) npm : 6.9.0 OS : macOS Mojave