Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

WIP feat/code-quality: Code Quality Improvements #390

Merged
merged 18 commits into from
Sep 18, 2019
Merged

WIP feat/code-quality: Code Quality Improvements #390

merged 18 commits into from
Sep 18, 2019

Conversation

johnpyp
Copy link
Contributor

@johnpyp johnpyp commented Aug 11, 2019

Description

This PR is meant to include a few different changes meant to improve code quality across the codebase (in the same spirit as the linting and formatting PR).

List of changes (subject to change):

  • Improves use of plural and singular nouns to differentiate arrays vs singular items
  • Improves namespace pollution to avoid often confusing variable name overriding.
  • Removes some unused packages and code ((Refactor) Delete zombie code #389)
  • Upgrades all packages to latest version
  • Changes some code structure to adhere to new 18.0 airbnb eslint config rules
  • Refactors the query-string usage
  • Adds a "Map View" and "Admin View" toggle button to the header to more seamlessly switch between the two.

Fixes #389
Fixes #328

Outstanding Issues

Motivation and Context

Code quality is important to maintain to ease development friction for existing contributors, as well as helping new contributors on-board quickly.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

johnpyp added 13 commits August 11, 2019 01:08
- Updated reactstrap, react-beautiful-dnd
- Fixed deprecation warnings for component methods
Uses setImmediate on query-string manipulation for add and remove, which
allows rendering to happen faster
- Removed qs-lite, an outdated dependency
- Refactored query-string usage for resources into a utils file
- Removed setImmediate, causing animation issues
@acharliekelly acharliekelly self-requested a review August 13, 2019 22:51
acharliekelly
acharliekelly previously approved these changes Aug 13, 2019
Copy link
Collaborator

@acharliekelly acharliekelly left a comment

Choose a reason for hiding this comment

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

Looks good - changes make sense, and app continues to work.

@acharliekelly
Copy link
Collaborator

Hey, maybe you want to split this into 2 PRs, one for each issue

@acharliekelly
Copy link
Collaborator

Is it possible to put the Map Toggle button in a separate PR? I think the other issues are close enough not to need their own PRs, but the more distinct PRs you have, the easier it will be to fix merge conflicts later

Copy link
Collaborator

@acharliekelly acharliekelly left a comment

Choose a reason for hiding this comment

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

I feel like there should be issues addressed per PR, but that's more of a general concept rather than a critique of this particular PR. It works, and I really want to use the changes to fix other issues, so I'm approving. If it blows up later, my bad.

@acharliekelly acharliekelly merged commit 0b62e4f into codeforboston:development Sep 18, 2019
acharliekelly pushed a commit that referenced this pull request Sep 18, 2019
* Fixed plural and singular use in redux

* More plural changes

* Removed dead index.css file

* Updated some packages, and removed ^ from all packages

* Updated packages and fixed deprecation

- Updated reactstrap, react-beautiful-dnd
- Fixed deprecation warnings for component methods

* Updated eslint

* Manually linted to align with new rules

* Removed unused packages

* Update more packages, fix sorting bug

* Removed unused mapbox-gl, updated react testing

* Improved OrganizationCard performance

Uses setImmediate on query-string manipulation for add and remove, which
allows rendering to happen faster

* Removed qs-lite, refactored query-string

- Removed qs-lite, an outdated dependency
- Refactored query-string usage for resources into a utils file
- Removed setImmediate, causing animation issues

* Added source-map-explorer to analyze bundle size

* Admin view and Map View toggle button to more easily build your saved resources page

* Added a localstorage entry for hideFeedback to remind in a week, rather than on refresh

* Fixed redux mutated state bug in CategoryList

* Fixed mutating bug... again
RajChaudhry added a commit that referenced this pull request Jan 23, 2020
* Update README.md

* For backwards compatibility, /admin should redirect to /revere/admin (#305)

* Ensure that header and page height add to 100% (#312)

* Did some basic "boyscouting", deleted a lot of unused/commented code (#318)

Basic housekeeping stuff. Part of my effort to clean up and refactor the code according to this guide: https://dev.to/gonedark/10-practices-for-readable-code-143a . This addresses the first two points, code formatting and deleting dead code. Also followed some "boyscouting" guidelines seen here: https://jasonmccreary.me/articles/are-you-a-boy-scout//

* Show cursor pointer for </a>

* Notfoundpage #306 (#311)

* fix(error-page): implement NotFoundPage on 404

remove console log

show NotFoundPage on correct path but wrong route => revere/dewdfewfwf returns NotFoundPage

fix(notfoundpage): fix app rendering notfound on all resource pages

fix(404-page): fix 404 on pages not found

* fix(AppContainer): handle 404

* fix(404): disabled 404 on :resource/admin page

* Change categoryautosortscript to category (#324)

* Bump macaddress from 0.2.8 to 0.2.9 (#322)

Bumps [macaddress](https://github.com/scravy/node-macaddress) from 0.2.8 to 0.2.9.
- [Release notes](https://github.com/scravy/node-macaddress/releases)
- [Commits](scravy/node-macaddress@0.2.8...0.2.9)

Signed-off-by: dependabot[bot] <[email protected]>

* Rename category column (#332)

* Change categoryautosortscript to category

* Change "category" to "categories"

* fixed categories label bug. (#333)

* fixed categories label bug.

* added legacy capability with category autosortscript so it works with MSM and other test templates.
once we merge development and master, we will change the revere sheet column D to "Categories", so we can still demo the master branch without too many problems.

* Updated readme with admin/client views

Explained basic differences btw admin/client views and provided example links

* Update README.md

fixed enumerating list

* Sorting selection box improvements (#320)

* Make footer displace main page instead of drawing over it (#336)

* Make footer displace main page instead of drawing over it

* Remove experimental code

* deploying again (#343) (#344)

* Update README.md

* For backwards compatibility, /admin should redirect to /revere/admin (#305)

* Ensure that header and page height add to 100% (#312)

* Did some basic "boyscouting", deleted a lot of unused/commented code (#318)

Basic housekeeping stuff. Part of my effort to clean up and refactor the code according to this guide: https://dev.to/gonedark/10-practices-for-readable-code-143a . This addresses the first two points, code formatting and deleting dead code. Also followed some "boyscouting" guidelines seen here: https://jasonmccreary.me/articles/are-you-a-boy-scout//

* Show cursor pointer for </a>

* Notfoundpage #306 (#311)

* fix(error-page): implement NotFoundPage on 404

remove console log

show NotFoundPage on correct path but wrong route => revere/dewdfewfwf returns NotFoundPage

fix(notfoundpage): fix app rendering notfound on all resource pages

fix(404-page): fix 404 on pages not found

* fix(AppContainer): handle 404

* fix(404): disabled 404 on :resource/admin page

* Change categoryautosortscript to category (#324)

* Bump macaddress from 0.2.8 to 0.2.9 (#322)

Bumps [macaddress](https://github.com/scravy/node-macaddress) from 0.2.8 to 0.2.9.
- [Release notes](https://github.com/scravy/node-macaddress/releases)
- [Commits](scravy/node-macaddress@0.2.8...0.2.9)

Signed-off-by: dependabot[bot] <[email protected]>

* Rename category column (#332)

* Change categoryautosortscript to category

* Change "category" to "categories"

* fixed categories label bug. (#333)

* fixed categories label bug.

* added legacy capability with category autosortscript so it works with MSM and other test templates.
once we merge development and master, we will change the revere sheet column D to "Categories", so we can still demo the master branch without too many problems.

* Updated readme with admin/client views

Explained basic differences btw admin/client views and provided example links

* Update README.md

fixed enumerating list

* Sorting selection box improvements (#320)

* Make footer displace main page instead of drawing over it (#336)

* Make footer displace main page instead of drawing over it

* Remove experimental code

* Fix tel link (#346)

* 📝 Update Documentation and Templates for Contributors (#355)

* Initial commit of docs with templates and updated CONTRIBUTING guide

* Udpate README for clarity and contributing

* Remove old pull request template

* Update section formatting

* Remove FAQ section until needed

* Update badge centering and remove whitespace

* Update text layout and description text link

* Update broken link and title

* feat: modal replacement (#335)

* Make 'add resource' button toggle add/remove (#361)

* Revert "Make 'add resource' button toggle add/remove (#361)" (#363)

This reverts commit b1a69fd.

* Add correct label to sortbar on the Admin Page (#365)

* Update Contributing Guide(s) Syntax, Style, and Content  (#360)

* Remove old CONTRIBUTING.md

* Updated link to good first issues in contributing guide

* Add additional change category to pull request template

* Make the add-resource button toggle (#364)

* fixed undefined tags bug (#368)

* refactor(entire project): add refactor/css on top on development (#372)

* refactor(entire project): add refactor/css on top on development

* fix(SavedResource): remove function binding in constructor

* chore(yarn): add yarn.lock

* chore(yarn): commit yarn.lock

* chore(vscode settings): add settings.json

* fix(css): implement css fixes

* chore(docs): implement docs folder; add todo on savebutton

* chore(vscode settings): disable auto format

* Fixed Community Connect 'continue' button (#380)

- Changes the 'continue' button to a router link button
- Adds a clearSavedResources redux action and reducer to clear saved
  resources without needing a refresh

* Redux nested state fix and readability improvements (#375)

* Nested state bug fixed, array spread consistency

- The bug with nested state when adding a saved resource is fixed

- Object.assign has been replaced with array spread syntax ([...array1,
  ...array2]) to improve consistency and readability.

* Performance and readability in CardGrid

The mapStateToProps function in the CardGrid component was using a
double for-loop to filter valid results. This fixes that issue by using
a Set, which is O(1) time complexity, so the overall time complexity of
the loop is now O(n) rather than O(n^2).

Additionally, readability is improved by using less code with more
readable apis like map and filter instead of vanilla for loops.

* feat(css): add button animation, rework saved resources (#381)

* feat(css): add button animation, rework saved resources

* remove container-fluid

* feat(loading screen): add custom loading screen

* refactor(appcontainer): remove isfetching conditional in render return

* refactor(app container): break up components

* modify saved resources css

* Added eslint with airbnb config

* Improved eslint config and deps

* Upgraded react-scripts and some dependencies

* Fixed minor Loading bug

* Pinned and updated some react dependencies

* Autofix eslint

* Added prettier with eslint integration

* manually fixed some stuff

* Improved eslint and prettier integration

* Explicitly added recompose to dependencies

* Linted all manual errors

Top changes:
- Added missing propTypes to components
- Export default over named export when only one export
- Possibly confusing namespace collisions

* Fixed some outstanding bugs

* Minor changes

- Removed @emotion dependencies
- Removed NotFoundPageLayout dead code
- Changed a method on SavedResourcesContainer to an arrow function

* Added lint-staged and husky for automatic linting

* Added trailing commas config to prettier

* Autofixed trailing commas

* Fixed accidental bug in OrganizationMarker

* Clear all categories (#391)

* Refactored handleChange and filter resource

1. TODO have filteredResource outside the handle change so we can
handle the selectedCategory state correctly
1. Right now the selectedCategory gets added everytime the check-
box is clicked.

* Refactor lifecycle methods and change styling

- Refactored filter categories
- Replaced checkboxes with list items
- Sort categories alphabetically

* Include proptypes and fix export issue

1. updated proptypes
2. fixed styling using the linter

* Fixed proptypes and included default prop types

* Move clear categories button below list

* WIP feat/code-quality: Code Quality Improvements (#390)

* Fixed plural and singular use in redux

* More plural changes

* Removed dead index.css file

* Updated some packages, and removed ^ from all packages

* Updated packages and fixed deprecation

- Updated reactstrap, react-beautiful-dnd
- Fixed deprecation warnings for component methods

* Updated eslint

* Manually linted to align with new rules

* Removed unused packages

* Update more packages, fix sorting bug

* Removed unused mapbox-gl, updated react testing

* Improved OrganizationCard performance

Uses setImmediate on query-string manipulation for add and remove, which
allows rendering to happen faster

* Removed qs-lite, refactored query-string

- Removed qs-lite, an outdated dependency
- Refactored query-string usage for resources into a utils file
- Removed setImmediate, causing animation issues

* Added source-map-explorer to analyze bundle size

* Admin view and Map View toggle button to more easily build your saved resources page

* Added a localstorage entry for hideFeedback to remind in a week, rather than on refresh

* Fixed redux mutated state bug in CategoryList

* Fixed mutating bug... again

* fix social media links (#399)

* Copy Button on SavedResources Panel (#405)

* add margin to buttons

* add copy button

* increase space between buttons

* use hooks to change icon & title on click

* Implemented print page (#407)

* Implemented print page

* Removed console log

* Added print icon

* Added website

* Removed console log

* 402 space in distance (#404)


* Update OrganizationCardSocialMedia.js

* Add space in distance for each organization card
when running on development.

* Bump tar from 2.2.1 to 2.2.2 (#410)

Bumps [tar](https://github.com/npm/node-tar) from 2.2.1 to 2.2.2.
- [Release notes](https://github.com/npm/node-tar/releases)
- [Commits](isaacs/node-tar@v2.2.1...v2.2.2)

Signed-off-by: dependabot[bot] <[email protected]>

* Bump mixin-deep from 1.3.1 to 1.3.2 (#409)

Bumps [mixin-deep](https://github.com/jonschlinkert/mixin-deep) from 1.3.1 to 1.3.2.
- [Release notes](https://github.com/jonschlinkert/mixin-deep/releases)
- [Commits](jonschlinkert/mixin-deep@1.3.1...1.3.2)

Signed-off-by: dependabot[bot] <[email protected]>

* Bump eslint-utils from 1.4.0 to 1.4.3 (#408)

Bumps [eslint-utils](https://github.com/mysticatea/eslint-utils) from 1.4.0 to 1.4.3.
- [Release notes](https://github.com/mysticatea/eslint-utils/releases)
- [Commits](mysticatea/eslint-utils@v1.4.0...v1.4.3)

Signed-off-by: dependabot[bot] <[email protected]>

* Add and use Collapsible for the categories (#403)

* Add and use Collapsible fir the categories

* first attempt at styling collapseable

* round edges

Co-authored-by: ndesai06 <[email protected]>
Co-authored-by: galia traub <[email protected]>
Co-authored-by: Jason Zhao <[email protected]>
Co-authored-by: Shoubhik Bose <[email protected]>
Co-authored-by: David Ko <[email protected]>
Co-authored-by: Charlie Kelly <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tim Christovich <[email protected]>
Co-authored-by: John Paul Penaloza <[email protected]>
Co-authored-by: Merritt Blanks <[email protected]>
Co-authored-by: Khoawala <[email protected]>
Co-authored-by: Lucas Chartier <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants