Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Table align not working #28

Closed
craftzdog opened this issue Oct 21, 2016 · 12 comments
Closed

Table align not working #28

craftzdog opened this issue Oct 21, 2016 · 12 comments
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem

Comments

@craftzdog
Copy link
Contributor

craftzdog commented Oct 21, 2016

React ignores align property since this property is obsolete in HTML5.
align is not listed as supported attribute: https://facebook.github.io/react/docs/tags-and-attributes.html
It should use CSS instead.

@wooorm
Copy link
Member

wooorm commented Oct 23, 2016

Interesting, was it dropped recently or never supported?

I guess we best add classes in that case, right? align-left, align-center, align-right?
Inline styles seems like a bad idea.

@craftzdog
Copy link
Contributor Author

craftzdog commented Oct 23, 2016

I don't know when React started to ignore it.
Adding classes looks OK for me.

@craftzdog
Copy link
Contributor Author

But adding classes requires CSS styling in every application.

@wooorm
Copy link
Member

wooorm commented Oct 23, 2016

I’d like to get someone else’s input on this as well, but I’m OK with requiring CSS: it’s better than inline styles at least.

@craftzdog
Copy link
Contributor Author

found a way to avoid this issue using remarkReactComponents:

import React, { createElement } from 'react'
import remark from 'remark'
import remarkReact from 'remark-react'

function createTableCellComponent (tagName) {
  return class TableCell extends React.Component {
    render () {
      const style = { textAlign: this.props.align }
      const props = { ...this.props, style }
      return createElement(tagName, props, this.props.children)
    }
  }
}

const options = {
  remarkReactComponents: {
    td: createTableCellComponent('td'),
    th: createTableCellComponent('th')
  }
}
const processor = remark().use(remarkReact, options)

@wooorm
Copy link
Member

wooorm commented Nov 8, 2016

@tmcw Any thoughts on this?

  • Should align be ignored in remark-react?
  • Should it be passed through?
  • Should we expose align as classes by default?

@tmcw
Copy link
Contributor

tmcw commented Nov 8, 2016

I lean toward inline styles here - if we required a stylesheet for remark-react's output to be viewed correctly, I doubt many would include it. And GFM supports table alignment, so it seems like a complete implementation should include it.

@jmca
Copy link

jmca commented Dec 28, 2016

I am running up against this issue creating email templates using React. Unfortunately I need to use many align attributes for legacy reasons.

@wooorm
Copy link
Member

wooorm commented Feb 27, 2017

Hey folks! I try’d my hand at this after GH-32, by using style instead of align, but I forgot that the sanitation mechanism strips style tags away (as it’s not very safe to have style, just like classes, render by default).

I think it would be simplest to have a component handle this instead, something like remark-react-lowlight.

@noradaiko Would you be interested in creating something like that? (or maybe someone else?)

@craftzdog
Copy link
Contributor Author

craftzdog commented Feb 28, 2017

Do you mean that having a separate component from remark-react for it is good idea?
I don't like it because the table align support is standard in GFM.

I think it's better to have a default remarkReactComponents for the table elements.
It can be added here: https://github.com/mapbox/remark-react/blob/master/index.js#L36

Thanks!

@wooorm
Copy link
Member

wooorm commented Feb 28, 2017

@noradaiko I think you’re right! I just pushed to GH-32 to use remarkReactComponents, and fix this!

@craftzdog
Copy link
Contributor Author

Excellent!

@tmcw tmcw closed this as completed in c91d4cd Mar 2, 2017
@wooorm wooorm added ⛵️ status/released 🐛 type/bug This is a problem 👶 semver/patch This is a backwards-compatible fix 🗄 area/interface This affects the public interface labels Aug 14, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem
Development

No branches or pull requests

4 participants