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

fix Announcements component has some warning on React #17

Merged
merged 2 commits into from
Jul 1, 2017

Conversation

takayamaki
Copy link
Member

@takayamaki takayamaki commented Jun 21, 2017

related #13

だいたいは #13 に書いてある fvh-P@3fa862e をベースにしてます。

IconButtonのtitle属性に使うI18nメッセージは本来であれば専用のものを用意すべきなんだろうと思われますが、
全く同じ用途のものが重複してしまって面倒くさくなりそうだったのでmedia_galleryのものをそのまま拝借しました

@takayamaki takayamaki requested a review from lnanase June 21, 2017 23:11
return React.createElement('br');
return (
<br key={i} />
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brタグにkey属性が抵抗ありましたので

        return React.createElement('br', { key: i });

でいかがでしょうか。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(ただの野次馬です)普通に String#replace 使って書けないもんでしょうか?

   nl2br (text) {
     return text.replace(/\n/<br>/gm);
   }

みたいな(試してはないです)。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/String/replace

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsx として認識してくれないんですかね?
Reactiveにする必要がないところにまで keyつけるの変かなーていうのはあります 😥

Copy link
Member

@treby treby Jun 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いろいろ調べた結果、裏技っぽいものは見つけたw https://facebook.github.io/react/docs/dom-elements.html#dangerouslysetinnerhtml

i18n
"welcome.message": "初めましての方へ<br />いらっしゃいませ。{domain}は全てのアイ...

jsx
<p dangerouslySetInnerHTML={{ __html: intl.formatMessage(messages.welcome, { domain: document.title }) }} />

現実的には、要素を分解して配置してあげるのが良いのですかね?野次終わり。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dangerouslySetInnerHTML はHTMLをそのまま出力なのでまぁ使いたくないってのはありますw

  • return text.replace(/\n/g, '<br>');
    <br>で出力される

  • return text.replace(/\n/<br>/gm);
    Push rejected, failed to compile Ruby app.

  • return text.replace(/\n/g, <br>);
    Push rejected, failed to compile Ruby app.

  • return text.replace(/\n/g, React.createElement('br'));
    [object Object]で出力される

Thank you.

@takayamaki takayamaki merged commit 88d6aae into imastodon Jul 1, 2017
@takayamaki takayamaki deleted the fix_react_warnings branch July 1, 2017 05:01
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.

3 participants