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.

Sign up for GitHub

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

Jump to bottom

(#97, #98) Update app module architecture with NCIDS components #99

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

(#97, #98) Update app module architecture with NCIDS components #99

wants to merge 1 commit into from

Conversation

adriancofie
Copy link
Contributor

@adriancofie adriancofie commented Jul 14, 2023 •
edited
Loading

(#97, #98) Update app module architecture with NCIDS components

  • Update header and footer within index.html
  • Update webpack config sass-loader
  • Add ncids-application-page.scss
  • Add reference to cancer.gov ncids-minimal.css stylesheet within index.html
  • Connect toggle / dropdown select to translation
  • Add ncids-application-page.scss and updated webpack.config file to configure sass-loader with added load paths
  • Update css-loader from 3.4.2 to 5.2.7, postcss-loader from 3.0.0 to 4.3.0
  • Downgrade sass-loader from 10.4.1 t0 10.2.0
  • Update webpack.config
  • Remove reference to cancer.gov ncids-minimal.css stylesheet within index.html
  • Create entrypoint in webpack.config that adds ncids-minimal.css and ncids-application-page.css to app
  • Update current template to use ncids grid
  • Add mobile menu
  • Add tests for SideNav and ItemDetail view
  • Add svg-loader config to webpack
  • Fix typo and pathing in getSampleCallResults which was causing api call failure
  • Update PageNotFound error page
  • Remove all previous postcss related packages and replaced with postcss, postcss-loaderandautoprefixer`
  • Add fonts to index.html
  • Revert changes in PageNotFound. To be reconstructed in line with NCIDS layout/styling
  • Add back postcss-safe-parser and postcss-prefix-selector
  • Remove generate-cgdp-legacy.js file along with references in package.json to run build script, and postcss-prefix-selector which was used in the file.
  • Remove HomeNoSideNav component and all references
  • Update PageNotFound component to conform with NCIDS styling
  • Update unit tests for PageNotFound
  • Fix missing base path for page not found link
  • Add basepath to apiEndpoint used to initiate app in index.js for react-app-dev.cancer.gov

Closes #97

  • Update supported browsers browserslist property in package.json

Closes #98

@github-actions
Copy link

Viewing Information

@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 4d2277a to 23af355 Compare July 17, 2023 19:05
@seyilonge-nci seyilonge-nci force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 23af355 to 18b1563 Compare July 20, 2023 15:41
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch 12 times, most recently from 5b648e3 to 9e35e6d Compare August 2, 2023 18:06
@seyilonge-nci seyilonge-nci force-pushed the ticket/97-Add-NCIDS-to-react-app branch 3 times, most recently from 30732b1 to 2babd26 Compare August 17, 2023 19:26
@belaolsonNIH belaolsonNIH force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 2babd26 to 73d1035 Compare August 18, 2023 17:46
@seyilonge-nci seyilonge-nci marked this pull request as ready for review August 28, 2023 11:24
@seyilonge-nci seyilonge-nci force-pushed the ticket/97-Add-NCIDS-to-react-app branch 3 times, most recently from 1641a92 to 72a2320 Compare August 31, 2023 13:21
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 72a2320 to 51b17a2 Compare September 12, 2023 14:04
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 892d0cd to 51b17a2 Compare September 15, 2023 14:44
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch 3 times, most recently from e44356a to 39eacaf Compare October 10, 2023 20:34
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch 2 times, most recently from 7f3fadf to 6d31c74 Compare October 10, 2023 21:02
@adriancofie adriancofie changed the title (#97) Update app module architecture with NCIDS components (#97, #98) Update app module architecture with NCIDS components Oct 10, 2023
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 6d31c74 to 047255f Compare October 10, 2023 21:04
sarinapadilla
sarinapadilla approved these changes Nov 13, 2023
View reviewed changes
Copy link

@sarinapadilla sarinapadilla left a comment

Choose a reason for hiding this comment

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

This looks good on react-app-dev!

@belaolsonNIH belaolsonNIH force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 6edbfdd to 124714e Compare April 29, 2024 17:52
arcepaul
arcepaul reviewed May 7, 2024
View reviewed changes
package.json Outdated
"sass": "^1.54.9",
"sass-loader": "^10.3.1",
"sass": "^1.63.6",
"sass-loader": "^10.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bryan pointed out that this package is downgrading?

arcepaul
arcepaul reviewed May 7, 2024
View reviewed changes
src/hooks/customFetch.js Outdated
@@ -1,7 +1,7 @@
import { useEffect, useState } from 'react';
import { useQuery } from 'react-fetching-library';

export const useCustomQuery = (action, shouldFetch = true) => {
export let useCustomQuery = (action, shouldFetch = true) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why the switch to let here?

arcepaul
arcepaul reviewed May 7, 2024
View reviewed changes
src/styles/ncids-application-page.scss Outdated
@include u-font-family('body');
margin-left: 10px;
padding-top: 4px;
padding-bottom: 4px;
Copy link
Contributor

Choose a reason for hiding this comment

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

this file needs to be formatted

@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 124714e to 621ad5e Compare July 22, 2024 21:14
@adriancofie
(#97, #98) Update app module architecture with NCIDS components
c90663f 
* Update header and footer within index.html
* Update webpack config sass-loader
* Add ncids-application-page.scss
* Add reference to cancer.gov ncids-minimal.css stylesheet within index.html
* Connect toggle / dropdown select to translation
* Add `ncids-application-page.scss` and updated `webpack.config` file to configure sass-loader with added load paths
* Update `css-loader` from `3.4.2` to `5.2.7`, `postcss-loader` from `3.0.0` to `4.3.0`
* Downgrade `sass-loader` from `10.4.1` t0 `10.2.0`
* Update `webpack.config`
* Remove reference to cancer.gov ncids-minimal.css stylesheet within index.html
* Create entrypoint in `webpack.config` that adds `ncids-minimal.css` and `ncids-application-page.css` to app
* Update current template to use ncids grid
* Add mobile menu
* Add tests for SideNav and ItemDetail view
* Add `svg-loader` config to webpack
* Fix typo and pathing in `getSampleCallResults` which was causing api call failure
* Update `PageNotFound` error page
* Remove all previous postcss related packages and replaced with `postcss`, postcss-loader` and `autoprefixer`
* Add fonts to `index.html`
* Revert changes in `PageNotFound`. To be reconstructed in line with NCIDS layout/styling
* Add back `postcss-safe-parser` and `postcss-prefix-selector`
* Remove `generate-cgdp-legacy.js` file along with references in `package.json` to run build script, and `postcss-prefix-selector` which was used in the file.
* Remove `HomeNoSideNav` component and all references
* Update `PageNotFound` component to conform with NCIDS styling
* Update unit tests for `PageNotFound`
* Fix missing base path for page not found link
* Add basepath to `apiEndpoint` used to initiate app in `index.js` for `react-app-dev.cancer.gov`
* Added `ncids-react` alpha package for POC

Closes #97

* Update supported browsers browserslist property in package.json

Closes #98
@adriancofie adriancofie force-pushed the ticket/97-Add-NCIDS-to-react-app branch from 621ad5e to c90663f Compare July 22, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcepaul arcepaul arcepaul left review comments

@sarinapadilla sarinapadilla sarinapadilla approved these changes

@seyilonge-nci seyilonge-nci Awaiting requested review from seyilonge-nci

@belaolsonNIH belaolsonNIH Awaiting requested review from belaolsonNIH

Assignees
No one assigned
Labels
Needs Technical Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enabler: Update browsers list Story: Update app module architecture with NCIDS components
3 participants
@adriancofie @sarinapadilla @arcepaul