Skip to content
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

content: --inner-min-width is missing from ion-item's CSS Custom Properties #3362

Closed
chawes13 opened this issue Jan 3, 2024 · 10 comments
Closed
Labels
needs: reply Issues that need more information from the author

Comments

@chawes13
Copy link
Contributor

chawes13 commented Jan 3, 2024

URL

https://ionicframework.com/docs/api/item#css-custom-properties-1

Issue Description

Documentation for recently added --inner-min-width for ion-item is missing. This can be resolved by adding an @prop --inner-min-width comment in the JSDoc comment of https://github.com/ionic-team/ionic-framework/blob/fbada1d1703cfea87224ced56eaf39caf9322763/core/src/components/item/item.scss

PR for newly added property: ionic-team/ionic-framework#28631

@chawes13 chawes13 added the triage New issues label Jan 3, 2024
@averyjohnston
Copy link
Contributor

Thank you for the issue. This property was intentionally left undocumented as noted in the PR description:

I made this a CSS variable but left it undocumented. If developers need a way of changing this min-width they can request it and we can easily expose this variable. However, I think 4rem is small enough that this should be sufficient.

Did you have a use case for changing the prop's value?

@averyjohnston averyjohnston added the needs: reply Issues that need more information from the author label Jan 5, 2024
@ionitron-bot ionitron-bot bot removed the triage New issues label Jan 5, 2024
@chawes13
Copy link
Contributor Author

chawes13 commented Jan 8, 2024

Ah, shoot sorry about that! I missed that comment.

Yes, when I upgraded my app, I had a styling regression that required overriding the property (recording: https://share.zight.com/z8uB7A0K).

The variable does seem to be exposed already (i.e., I can override it) even though it's undocumented?

@ionitron-bot ionitron-bot bot added triage New issues and removed needs: reply Issues that need more information from the author labels Jan 8, 2024
@averyjohnston
Copy link
Contributor

Could you post a Stackblitz app or Github repo showing the issue you're facing (with only the code necessary to reproduce the problem)?

@averyjohnston averyjohnston added the needs: reply Issues that need more information from the author label Jan 9, 2024
@ionitron-bot ionitron-bot bot removed the triage New issues label Jan 9, 2024
@chawes13
Copy link
Contributor Author

@amandaejohnston Sure! Is there a Stackblitz Ionic starter template or a good recent example to fork?

@ionitron-bot ionitron-bot bot added triage New issues and removed needs: reply Issues that need more information from the author labels Jan 11, 2024
@averyjohnston
Copy link
Contributor

You can make a blank starter app. I'll label this as "needs reproduction" so the info shows up 👀

Copy link

ionitron-bot bot commented Jan 11, 2024

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that we are not able to reproduce.

Please provide easy to follow steps for us to reproduce this issue.

@ionitron-bot ionitron-bot bot removed the triage New issues label Jan 11, 2024
@averyjohnston
Copy link
Contributor

Oh, I forgot the docs repo has a different template for that 😆 The contributing guide has some tips: https://ionicframework.com/docs/contributing/how-to-contribute#how-to-create-a-reproduction

@chawes13
Copy link
Contributor Author

@amandaejohnston Here you go! https://stackblitz.com/~/github.com/chawes13/ionic-min-padding-repro

You'll notice that the social media links in the menu are nicely aligned. If you edit the package.json to use ^7.6.0 for @ionic/react and @ionic/react-router and remove the placeholder prop on line 31 of App.tsx, you'll see that the padding changes and the icons overflow the container (i.e., introduces a styling regression).

If you navigate to src/pages/Home.css, you can uncomment the override of inner-min-width, which fixes the styling issue.

@averyjohnston
Copy link
Contributor

Thank you! I did some digging internally, and as it turns out, we're actually looking at removing the --inner-min-width property as part of a separate fix in this PR: ionic-team/ionic-framework#28773 Here is a dev build you can use to try this fix:

npm install @ionic/react@7.6.5-dev.11704916749.1e64a3a7 @ionic/react-router@7.6.5-dev.11704916749.1e64a3a7

I tried this with your provided repro and it seems to have resolved things, but could you give it a try on your end and let me know if you encounter any trouble?

@averyjohnston averyjohnston added the needs: reply Issues that need more information from the author label Jan 17, 2024
@ionitron-bot ionitron-bot bot removed the triage New issues label Jan 17, 2024
Copy link

ionitron-bot bot commented Jan 31, 2024

Thanks for the issue! This issue is being closed due to the lack of a reply. If this is still an issue, please create a new issue and ensure the template is fully filled out.

Thank you for using Ionic!

@ionitron-bot ionitron-bot bot closed this as completed Jan 31, 2024
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: reply Issues that need more information from the author
Projects
None yet
Development

No branches or pull requests

2 participants