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

Replace inline styles with docs-container class #463

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Replace inline styles with docs-container class #463

wants to merge 1 commit into from

Conversation

elwayman02
Copy link
Contributor

Our own demo app uses a class instead of inline styles, but the guides never got updated. This will fail linting in most apps, and since we already provide the class for it, we should just tell users to use it.

Our own demo app uses a class instead of inline styles, but the guides never got updated. This will fail linting in most apps, and since we already provide the class for it, we should just tell users to use it.
@RobbieTheWagner
Copy link
Member

@elwayman02 I think there was some issue posted awhile ago and this was changed to be inline for some reason. @samselikoff do you recall?

@samselikoff
Copy link
Contributor

Yeah the doc-* classes are meant to be private to AD. They're not part of the public API (we've changed the classes before so if folks relied on them it would tie our hands).

@elwayman02
Copy link
Contributor Author

Hmm, then perhaps we should have some sort of docs-container component instead? The problem atm is that the default linting rules include no-inline-styles, so suggesting that users copy this code into their project is just going to create a headache they have to address immediately before committing.

@elwayman02
Copy link
Contributor Author

All that said, if this project is in fact going into maintenance mode, it seems unlikely that the docs-container class is going to mutate again in a significant way, so it might make sense to just merge this PR as-is and file an issue to address this another way if we think there's a better solution that would take more work.

@samselikoff
Copy link
Contributor

We should definitely add a <DocsContainer /> component, I think that would at least provide some encapsulation b/w the low level Tailwind styles + the high-level API.

@samselikoff
Copy link
Contributor

Didn't realize no-inline-styles was a default rule (definitely wasn't when we first wrote this guide), so could also use a class.

Really meant to add a Styling guide to explain that AD components are styled but the rest of the styles in the app are up to you to do it in userland however you like.

@elwayman02
Copy link
Contributor Author

Yea, it look like that rule was added to the default ruleset when Rob was prepping for the 1.0.0 release of ember-template-lint: ember-template-lint/ember-template-lint#457

@RobbieTheWagner
Copy link
Member

@elwayman02 mind rebasing this with master please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants