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

Split-pane uses invalid attribute for identifying main. #16583

Closed
jthoms1 opened this issue Dec 4, 2018 · 3 comments · Fixed by #16665
Closed

Split-pane uses invalid attribute for identifying main. #16583

jthoms1 opened this issue Dec 4, 2018 · 3 comments · Fixed by #16665
Assignees
Labels
package: core @ionic/core package package: react @ionic/react package

Comments

@jthoms1
Copy link
Contributor

jthoms1 commented Dec 4, 2018

You are required to add main boolean attribute to an HTML element to denote the target for ion-split-pane. The issue is that main is not a valid attribute for HTML elements. So if you are writing JSX and validating with Typescript then an error will be thrown because the boolean attribute does not exist in the type.

Currently this is an issue for React + Typescript, but I see this potentially be an issue for more people in the future. My suggestion would be that we provide an alternate way of defining the main panel.


Based on current discussions with @manucorporat I believe we came to the conclusion that we should continue to support the global html attributes for @ionic/angular projects but we should create classes for @ionic/vue and @ionic/react projects.

<div padding> would be come <div class="ion-padding">

Further team discussion is needed.

@jthoms1 jthoms1 added package: core @ionic/core package package: react @ionic/react package labels Dec 4, 2018
@manucorporat
Copy link
Contributor

This is not only about split-pane, there is a whole list of possible attributes:
https://github.com/ionic-team/ionic/blob/6776e65c9cae212a0bb2d7477b3a09beb2dfcad0/core/src/interface.d.ts#L59-L82

@jthoms1
Copy link
Contributor Author

jthoms1 commented Dec 6, 2018

Dang, I forgot about all of those. Currently React uses a global JSX definition that we can probably extend, but if they start using locally scoped JSX namespaces it will not longer be available to us.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-8.html

@ionitron-bot
Copy link

ionitron-bot bot commented Jan 10, 2019

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package package: react @ionic/react package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants