20 configurable project article querying (Medium part) - [merged]

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 00:31

_Merges 20_configurable_project_articlequerying -> master

Issue #20

Display articles from the C-hive Medium

Heroku: Turn on the dyno: Dashboard --> Domain ---> (Don't forget to turn off the dyno)

Changes:

• [x] create Article component (title, tags, date)
• [x] get data from medium (cors proxy, fetch, parse RSS)
• [x] create one, and two columns layouts
• [x] style article component to look nicer
• [x] add static font
• [x] create a ./setting dir for project settings or add with CI
• [x] make categories tags more readable for light and dark theme
• [x] setup styled-components media query
• [x] Badge component

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 00:34

added 1 commit

• 08a148e9 - article, categories, fonts, layouts related to #20

Compare with previous version

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 00:43

changed the description

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 09:00

added 1 commit

• 5bf0910f - update package.json related to #25

Compare with previous version

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:11

We should consider the configuration file thoroughly. First off, how about using a JSON based schema?

Example:

// => settings.json?
{
   "teamContributionCalendarUsers": {
      "gitHub": [],
      "gitLab": []
   }
}

Secondly, we should handle more than one Medium user, so let's define it as an array. Have you discussed it with @thisismydesign?

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

I'm not an expert in this field, but is it necessary to store all the other fonts besides the one that's being used here?

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

Please remove superfluous comments. :)

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

Please follow our naming conventions. Nevertheless, how about creating a badge component that accepts a text and background prop?

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

It's a good idea to clean up the dependencies, but then please remove these packages as well:

• babel-jest
• jest
• jest-fetch-mock

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

In general, these col components are defined differently:

• https://reactstrap.github.io/components/layout/

It'd be great to follow this convention, but we mustn't reinvent the wheel. :) So, what if we snatch a UI library instead of juggling with our self-created components? The choice is up to you.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

export const getMediumArticles = async () => {

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

import getProxyURL from "../GetProxyURL/GetProxyURL";

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

  const rssParser = new Parser();

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

This should be called differently as this also contains the user's comments besides its articles. Couldn't figure out a proper name though.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

import * as customHooks from "../../../utils/CustomHooks/CustomHooks";

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:12

import * as articlesUtils from "../../../utils/ArticlesUtils/ArticlesUtils";

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:13

Left a few comments. Please try to strive for following our naming conventions everywhere.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:14

I have just realized that we misunderstood each other in connection with the resources folder. :) Please place /src/themes into /src/resources/themes.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:18

I'm not sure if we plan to integrate other article portals. What if we name the file MediumUtils? This would also abbreviate the function name here, e.g. getArticles.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:19

Consider https://gitlab.com/c-hive/c-hive.dev/merge_requests/25#note_195139479.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 09:28

Branch naming:

• https://github.com/c-hive/guides/blob/master/universal/code-style.md#branches

Nevermind, it's correct.

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 10:17

Thanks, I miss that. Now I delete them with the right script.

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 10:44

Rename, and change js to JSON schema. :)

Not yet, I thought I'd wait until he came back, but I will leave this thread as unresolved.

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 10:45

changed this line in version 4 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 10:45

added 2 commits

• 0fbdb803 - update package.json related to #25
• 976800a7 - update settings.json related to #25

Compare with previous version

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 10:57

added 1 commit

• 5ff65e13 - update font styles, and delete fonts related to #25

Compare with previous version

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 11:03

I don't know which we will use, so leave all of them. But as you wrote I thought about it and now I just leave the basic three:

light(200), normal(400), bold(600)

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 11:41

Great suggestion, thanks.

[thisismydesign]

In GitLab by @thisismydesign on Jul 24, 2019, 11:51

Quick observations (didn't read everything):

No need to support multiple medium users at once. It's either a user or a publication.

The configuration should be external. I.e. should not stored in the repo. Since its quite complex I'm not in the favor of using many env vars. How about a json file which is added by our CI? Don't know if there's a nice way to do this (e.g. resource files in CI), the worst case it can be a command creating the file.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 24, 2019, 12:25

Consider @thisismydesign's comment.

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

changed this line in version 6 of the diff

[thisismydesign]

In GitLab by @nkapolcs on Jul 24, 2019, 14:38

added 1 commit

• b6e02646 - update Article component related to #25

Compare with previous version

[thisismydesign]

In GitLab by @gomorizsolt on Jul 25, 2019, 14:49

Extracting @thisismydesign's comment:

The configuration should be external. I.e. should not stored in the repo. Since its quite complex I'm not in the favor of using many env vars. How about a json file which is added by our CI? Don't know if there's a nice way to do this (e.g. resource files in CI), the worst case it can be a command creating the file.

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 19:39

marked the task make categories tags more readable for light and dark theme as completed

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 20:03

Thanks, I saw that, now its a JSON file in the settings folder, and I not really sure about why it is good if we add with CI. Can you explain it @thisismydesign?

Our main question was: we need to fetch data from multiple medium user, but @thisismydesign answered that.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 25, 2019, 20:20

(By the way, I mark as resolved thread above, please continue here.)

That's why the comment is extracted. :)

IMO this is the expected behavior. The users provide the configurations externally so that generate the site conveniently without writing/modifying any lines of code. But this is just my personal supposition, probably @thisismydesign can give a more precise answer.

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 20:21

I thought for using reactstrap, but I came to that conclusion if we need more, or complex UI elements, its a good idea using bootstrap or other UI framework. But now I stick with this solution to keep simpler and smaller the code, then later we can reconsider that.

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 20:33

Ah, ok, it was strange for me too, now it is in the good place. :)

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 20:37

Slowly, I try to give you fewer tasks in this regard. But thanks for noticing them.

[thisismydesign]

In GitLab by @gomorizsolt on Jul 25, 2019, 20:38

It's absolutely ok, no worries. :)

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 21:36

changed the description

[thisismydesign]

In GitLab by @thisismydesign on Jul 25, 2019, 22:08

In principle this project could be used by others as well, therefore the repo should not contain any data specific to us and the project should be configurable for any one person or organization. Having the config not part of the code I think is nicer and it makes it easier to follow upstream changes (no merge commit required on pulls).

[thisismydesign]

In GitLab by @nkapolcs on Jul 25, 2019, 23:57

Ok, thanks, it sounds reasonable. :)

[thisismydesign]

In GitLab by @gomorizsolt on Jul 26, 2019, 15:59

Outcome: #27.

As the PR already contains many changes, we ended up handling the tasks around the config file separately.