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

feat: port to app-platform #17

Merged
merged 24 commits into from
Mar 9, 2021
Merged

feat: port to app-platform #17

merged 24 commits into from
Mar 9, 2021

Conversation

mediremi
Copy link
Contributor

@mediremi mediremi commented Feb 24, 2021

Closes https://jira.dhis2.org/browse/DHIS2-10482

Changes

Required due to port

  • Replaced SCSS with CSS modules
  • Renamed componentWillReceiveProps to UNSAFE_componentWillReceiveProps and componentWillUpdate to UNSAFE_componentWillUpdate
  • Replaced deprecated React v15 PropTypes with @dhis2/prop-types
    • Added some missing PropTypes
  • Fixed import paths that were broken due to lack of webpack resolve plugin in app-platform
  • Updated jsoneditor in order to fix rendering issues with new React and styles
    • Also imported CSS from npm module instead of checked-in CSS file
  • Fixed componentWillReceiveProps of JSONEditor by shallow comparing previous props and current props instead of comparing by reference
    • Incorrect implementation led to broken tree view after updating React
  • Replaced custom spinner with CircularLoader from @dhis2/ui due to custom spinner using SCSS mixins for styling

Functional changes

  • Removed NavigationBar as it was superseded by app platform HeaderBar
  • Added eslint, prettier, husky, GitHub workflows
  • Removed unused/broken components and inaccessible routes (webapp/js/components/display/history/*.js and webapp/js/components/display/statistics/*.js)
    • Did not work due to missing Api method
  • Fixed all eslint issues
  • Updated material-ui from 0.16.4 to 0.19.0 in order to remove react-tap-event-plugin dependency
    • Changed onTouchTap handlers to onClick for all material-ui components
  • Added i18n support
  • Removed unused 'history' actions and reducers (see commit 4d37fbf)
  • Refactored Api class (old, new)
    • Removed unused methods and properties
    • Used async fns to make logic easier to follow
    • Extraced cache-related code to separate class
      • Fix cache implementation by using Map
        • Map has a .has(key) method, which does not suffer from the same problems as key in object (i.e. do not have to use hasOwnProperty, which may or may not exist and triggers a eslint error)
      • Added tests
  • Replaced this.fn = this.fn.bind(this) pattern in React class components to arrow functions

Further work to do in the future

  • Replace material-ui with @dhis2/ui
  • Replace d2 with @dhis2/app-runtime and remove @dhis2/app-runtime-adapter-d2
  • Setup Transifex
  • Replace react-router with react-router-dom

@mediremi mediremi marked this pull request as ready for review February 24, 2021 23:12
@mediremi mediremi requested a review from varl February 24, 2021 23:12
CHANGELOG.md Outdated Show resolved Hide resolved
LICENSE Show resolved Hide resolved
package.json Outdated
Comment on lines 1 to 36
},
"homepage": "https://github.com/dhis2/datastore-app",
"devDependencies": {
"babel-core": "^6.17.0",
"babel-eslint": "^7.1.1",
"babel-loader": "^6.2.5",
"babel-preset-es2015": "^6.16.0",
"babel-preset-react": "^6.16.0",
"babel-preset-stage-0": "^6.16.0",
"btoa": "^1.1.2",
"chai": "^3.5.0",
"css-loader": "^0.25.0",
"enzyme": "^2.6.0",
"eslint": "^3.10.2",
"eslint-config-dhis2": "^2.0.2",
"eslint-plugin-mocha": "^4.7.0",
"eslint-plugin-prefer-object-spread": "^1.1.0",
"eslint-plugin-react": "^4.1.0",
"html-webpack-plugin": "^2.30.1",
"ignore-styles": "^5.0.1",
"isomorphic-fetch": "^2.2.1",
"jsdom": "^9.8.3",
"json-loader": "^0.5.4",
"mocha": "^3.1.2",
"node-sass": "^4.13.1",
"react-addons-test-utils": "^15.4.1",
"react-bootstrap": "^0.30.7",
"react-hot-loader": "^3.0.0-beta.5",
"react-tools": "^0.13.3",
"redux-devtools": "^3.3.1",
"redux-logger": "^2.7.0",
"redux-mock-store": "^1.2.1",
"resolve-url-loader": "^1.6.0",
"sass-loader": "7",
"style-loader": "^0.13.1",
"url-loader": "^0.5.7",
"webpack": "^1.13.2",
"webpack-dev-server": "^1.16.2"
},
"dependencies": {
"babel-polyfill": "^6.23.0",
"d2": "^27.0.0-5",
"d2-manifest": "^1.0.0",
"d2-ui": "^27.0.0-6",
"d2-utilizr": "^0.2.9",
"flow-bin": "^0.33.0",
"jsoneditor": "^5.5.10",
"material-ui": "^0.16.4",
"react": "^15.4.0",
"react-chartjs-2": "1.5.1",
"react-dom": "^15.4.0",
"react-hot-loader": "^3.0.0-beta.5",
"react-json": "^0.2.1",
"react-json-inspector": "^7.0.0",
"react-json-pretty": "^1.2.1",
"react-redux": "^4.4.5",
"react-router": "3",
"react-tap-event-plugin": "^2.0.1",
"react-tools": "^0.13.3",
"redux": "^3.6.0",
"redux-devtools": "^3.3.1",
"redux-promise-middleware": "^4.1.0",
"redux-thunk": "^2.1.0",
"sprintf-js": "^1.0.3"
}
"name": "datastore-app",
"version": "1.0.0",
"description": "",
"license": "BSD-3-Clause",
"private": true,
"scripts": {
"build": "d2-app-scripts build",
"start": "d2-app-scripts start",
"test": "d2-app-scripts test",
"lint:js": "d2-style js check",
"lint:text": "d2-style text check",
"lint:staged": "yarn lint:js --staged && yarn lint:text --staged",
"lint": "yarn lint:js && yarn lint:text",
"format:js": "d2-style js apply",
"format:text": "d2-style text apply",
"format:staged": "yarn format:js --staged && yarn format:text --staged",
"format": "yarn format:js && yarn format:text"
},
"devDependencies": {
"@dhis2/cli-app-scripts": "^5.7.0",
"@dhis2/cli-style": "^7.3.0"
},
"dependencies": {
"@dhis2/app-runtime": "^2.7.0",
"@dhis2/app-runtime-adapter-d2": "^1.0.2",
"@dhis2/prop-types": "^2.0.3",
"d2": "^31.9.0",
"jsoneditor": "^9.2.0",
"material-ui": "0.19.0",
"react-redux": "^7.2.2",
"react-router": "3.0.5",
"react-router-dom": "^5.2.0",
"redux": "^4.0.5",
"redux-thunk": "^2.3.0"
}
Copy link
Member

Choose a reason for hiding this comment

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

These package.json diffs always make me happy! 🥇

@amcgee
Copy link
Member

amcgee commented Feb 25, 2021

Awesome work here @mediremi ! On first pass this looks really good. Huge PR though, from the commit messages it looks like there might be some functional changes or fixes mixed in here too... were those all issues caused by the port? Can you quickly describe any functional changes made here too? Thanks!!

@mediremi
Copy link
Contributor Author

@amcgee I've updated the PR description with a detailed breakdown of changes. Unfortunately the diff looks much bigger than it should be due to formatting and import changes causing git to show file renames as two separate files instead 😞 I should probably use git mv next time I port an app to the app platform 🤔

@mediremi mediremi requested a review from amcgee February 25, 2021 11:55
@mediremi mediremi merged commit 3012cb7 into master Mar 9, 2021
@mediremi mediremi deleted the fix/DHIS2-10482 branch March 9, 2021 15:02
dhis2-bot added a commit that referenced this pull request Mar 9, 2021
# [1.2.0](v1.1.0...v1.2.0) (2021-03-09)

### Features

* port to app-platform ([#17](#17)) ([3012cb7](3012cb7))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

mediremi added a commit that referenced this pull request Mar 11, 2021
* chore: add .github folder

* fix: remove scss

* fix: use @dhis2/prop-types

* fix: use onClick instead of onTouchTap

* fix: remove navigation bar

* fix: update jsoneditor to fix rendering

* refactor: remove unused routes

* fix: use i18n

* feat: add LICENSE.md

* refactor: remove history-related actions

* chore: remove react-router-dom dependency
mediremi added a commit that referenced this pull request Mar 11, 2021
* feat: port to app-platform (#17)

* chore: add .github folder

* fix: remove scss

* fix: use @dhis2/prop-types

* fix: use onClick instead of onTouchTap

* fix: remove navigation bar

* fix: update jsoneditor to fix rendering

* refactor: remove unused routes

* fix: use i18n

* feat: add LICENSE.md

* refactor: remove history-related actions

* chore: remove react-router-dom dependency

* feat: upgrade to @dhis2/ui v6 and bump other dependency versions  (#23)

* feat: upgrade to @dhis2/ui v6 and bump other dependency versions

* chore: add file extension to entry point in config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants