Posted From Feature #1122

Kennybll · 2017-11-28T04:56:03Z

Show the app used to publish a post in the UI.

Changes:

Adds a formatted message telling what app the post was posted by. Ex: Posted by busy
There is a tooltip around that, which says the version of that app.

Kennybll · 2017-11-28T04:56:39Z

From Workflow https://github.com/busyorg/busy/projects/6

Sekhmet · 2017-11-28T15:22:31Z

JSON.parse can fail, and when it does it throws error making React go wild.
Can you make it fail-safe?
https://gist.github.com/gucheen/12b90451f04733078d7f

bonustrack · 2017-11-28T15:42:46Z

@Kennybll here is my review:

The tag should appear next to the reputation indicator, now there is this long space due to Posted from steemit text width see:
The text Posted from steemit is too long et repetitive, i prefer just Steemit instead.
This should also appear in single post
We should save a list of apps in json file with name of app with correct case for example:

{
  steemit: 'Steemit',
  busy: 'Busy',
  esteem: 'eSteem',
  chainbb: 'chainBB',
}

Kennybll · 2017-11-28T16:52:23Z

Also fixes issue #1111.

bonustrack · 2017-11-28T16:54:34Z

@Kennybll in future please use more relevant name for your commits

Kennybll · 2017-11-28T16:55:05Z

Okay.

Sekhmet · 2017-11-28T16:57:48Z

@Kennybll

Make sure that your PRs do one thing and only one.
Don't integrate changes outside of base branch (master in this case) if it's not relevant to your PR.

Updates color property on __post_from

Kennybll · 2017-11-29T16:35:49Z

Again sorry for so many commit, they all are different files that are changed because Github.com doesn't allow multiple file commits. Git on my Windows laptop isn't working so well.

Kennybll · 2017-11-29T16:37:43Z

There @Sekhmet . Is there any other things you guys tend to not do, so I can be prepared in the future?
I remember you once said you put styles at the bottom of the imports.

Sekhmet

We don't have any written rules, but we try to make our codebase consistent. I think that code should speak for itself and that code is the best guideline.

If there is something related to code style that can be improved it should be discussed first and update an entire codebase to make sure everything is consistent.

I'm not sure what kind of git client are you using, but I think it should support committing multiple files. I recommend using command line git though.

Sekhmet · 2017-11-29T19:51:48Z

src/client/components/Story/PostedFromEmbed.js

+import { jsonParse } from '../../helpers/formatter';
+import './PostedFromEmbed.less';
+
+const PostedFromEmbed = ({ post }) => (


Why is this called PostedFromEmbed? Just PostedFrom makes sense in my opinion

Sekhmet · 2017-11-29T19:54:29Z

src/client/components/Story/PostedFromEmbed.js

+          <FormattedMessage
+            id="posted_from_tooltip"
+            defaultMessage={'Version: {version}'}
+            values={{ version: jsonParse(post.json_metadata).app.split('/')[1] }}


I don't think we need to parse metadata three times. We can do it just once. and save name and version in a variable.
This won't work too well if the app is no present (it will throw exception crashing React).

Sekhmet · 2017-11-29T19:55:43Z

src/client/components/Story/Story.js

+                    <FormattedRelative value={`${post.created}Z`} />
+                  </span>
+                </Tooltip>
+                <PostedFromEmbed


It's short enough to fit in one line.

Sekhmet · 2017-11-29T19:56:12Z

src/client/components/Story/Story.less

@@ -57,6 +57,10 @@
    padding: 16px;
    display: flex;

+    &__flex {


Is it used anywhere?

Yes. On line 238. I had to make that span display flex because the Topic tag would be on the next line next to the date posted.

Sekhmet · 2017-11-29T19:56:34Z

src/client/components/Story/StoryFull.js

@@ -311,7 +312,10 @@ class StoryFull extends React.Component {
                <FormattedRelative value={`${post.created}Z`} />
              </span>
            </Tooltip>
-            { Math.ceil(readingTime(post.body).minutes) > 1 &&
+            <PostedFromEmbed


This can fit on one line as well.

Sekhmet · 2017-11-29T19:57:25Z

src/client/components/Story/StoryFull.less

@@ -54,7 +54,12 @@
    position: relative;
    display: flex;
    padding: 16px 0;
-
+
+    &__post_from {


Looks like this isn't used as well.

Sekhmet · 2017-11-29T19:57:57Z

src/client/locales/en.json

-  "power_down_message": "Started power down: {value}"
+  "power_down_message": "Started power down: {value}",
+  "posted_from_tooltip": "Version: {version}",
+  "posted_from": "{from}"


I don't think we need translation for posted_from. It should just display app name.

Sekhmet · 2017-11-29T19:59:16Z

src/client/components/Story/PostedFromEmbed.js

+
+const PostedFromEmbed = ({ post }) => (
+  <span>
+    <span className="PostedFrom__bullet" />


CSS class name should have the same name as component PostedFormEmbed. It doesn't make a difference though as I think it should be renamed.

Sekhmet · 2017-11-29T20:00:32Z

src/client/components/Story/PostedFromEmbed.js

+import './PostedFromEmbed.less';
+
+const PostedFromEmbed = ({ post }) => (
+  <span>


I think it would make sense to add a class name to a topmost element in this component (to respect http://getbem.com/introduction guidelines).

Kennybll · 2017-11-29T21:33:55Z

@Sekhmet , so I figured out what was wrong with git on my computer. Nothing! Lol. I thought the process was hanging, but nope, it was running the pre-commit tests. It just took a minute to output something.

Kennybll · 2017-11-30T21:28:51Z

Is this good @Sekhmet @bonustrack ?

Sekhmet · 2017-12-01T10:19:50Z

src/client/components/Story/StoryFull.less

@@ -54,7 +58,7 @@
    position: relative;
    display: flex;
    padding: 16px 0;
-
+    


There is additional whitespace there.

bonustrack · 2017-12-01T10:24:14Z

src/client/helpers/apps.js

+  esteem: 'eSteem',
+  chainbb: 'chainBB',
+  utopian: 'Utopian',
+  dtube: 'dTube',


It's DTube, DSound and dMania (not dTube and dSound). Can you fix the case?

…ving extra space

Kennybll added 3 commits November 27, 2017 23:52

Posted From Feature

80551b4

Posted By Feature

33dc7f2

Posted From Feature

d7442d1

Merge branch 'master' into posted_from

609f8c4

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 05:56 Inactive

Merge branch 'master' into posted_from

632c939

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 14:34 Inactive

Posted From Feature

dd5af0f

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:49 Inactive

Posted From Feature

3fa6906

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:49 Inactive

Posted From Feature

0aef8b7

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:50 Inactive

Posted From Feature

221d47b

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:50 Inactive

Posted From Feature

85d2a28

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:51 Inactive

Posted From Feature

3a95c0a

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:51 Inactive

Merge branch 'master' into posted_from

eac7d3b

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 16:53 Inactive

Update For Past Pull Request

999f753

Updates color property on __post_from

bonustrack temporarily deployed to busy-master-pr-1122 November 28, 2017 17:01 Inactive

bonustrack temporarily deployed to busy-master-pr-1122 November 29, 2017 16:32 Inactive

Uses PostedFromEmbed Component - StoryFull.js

9011685

bonustrack temporarily deployed to busy-master-pr-1122 November 29, 2017 16:33 Inactive

Removes Unneeded Code From Story.less

c25ea42

bonustrack temporarily deployed to busy-master-pr-1122 November 29, 2017 16:33 Inactive

Sekhmet reviewed Nov 29, 2017

View reviewed changes

Refactors PostedFrom Component

965bf8a

bonustrack temporarily deployed to busy-master-pr-1122 November 29, 2017 21:32 Inactive

Merge branch 'master' into posted_from

7a45f0d

bonustrack temporarily deployed to busy-master-pr-1122 November 29, 2017 21:55 Inactive

Merge branch 'master' into posted_from

e654468

bonustrack temporarily deployed to busy-master-pr-1122 November 30, 2017 21:28 Inactive

Sekhmet reviewed Dec 1, 2017

View reviewed changes

bonustrack reviewed Dec 1, 2017

View reviewed changes

Updates adds.json to have correct letter case and StoryFull.less remo…

9b8f62b

…ving extra space

bonustrack temporarily deployed to busy-master-pr-1122 December 1, 2017 19:29 Inactive

Merge branch 'master' into posted_from

979c5c3

bonustrack temporarily deployed to busy-master-pr-1122 December 1, 2017 19:41 Inactive

Sekhmet approved these changes Dec 1, 2017

View reviewed changes

Merge branch 'master' into posted_from

0f547ed

bonustrack temporarily deployed to busy-master-pr-1122 December 1, 2017 20:36 Inactive

Merge branch 'master' into posted_from

53982ae

bonustrack deployed to busy-master-pr-1122 December 1, 2017 22:24 View deployment

bonustrack approved these changes Dec 2, 2017

View reviewed changes

bonustrack merged commit 3637e23 into busyorg:master Dec 2, 2017

bonustrack added this to the v2.2 milestone Dec 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Posted From Feature #1122

Posted From Feature #1122

Kennybll commented Nov 28, 2017

Kennybll commented Nov 28, 2017

Sekhmet commented Nov 28, 2017

bonustrack commented Nov 28, 2017

Kennybll commented Nov 28, 2017

bonustrack commented Nov 28, 2017

Kennybll commented Nov 28, 2017

Sekhmet commented Nov 28, 2017 •

edited

Loading

Kennybll commented Nov 29, 2017 •

edited

Loading

Kennybll commented Nov 29, 2017

Sekhmet left a comment

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Kennybll Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Sekhmet Nov 29, 2017

Kennybll commented Nov 29, 2017

Kennybll commented Nov 30, 2017

Sekhmet Dec 1, 2017

bonustrack Dec 1, 2017

Posted From Feature #1122

Posted From Feature #1122

Conversation

Kennybll commented Nov 28, 2017

Kennybll commented Nov 28, 2017

Sekhmet commented Nov 28, 2017

bonustrack commented Nov 28, 2017

Kennybll commented Nov 28, 2017

bonustrack commented Nov 28, 2017

Kennybll commented Nov 28, 2017

Sekhmet commented Nov 28, 2017 • edited Loading

Kennybll commented Nov 29, 2017 • edited Loading

Kennybll commented Nov 29, 2017

Sekhmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kennybll commented Nov 29, 2017

Kennybll commented Nov 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sekhmet commented Nov 28, 2017 •

edited

Loading

Kennybll commented Nov 29, 2017 •

edited

Loading