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

Manual check #168

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Manual check #168

wants to merge 3 commits into from

Conversation

collinschreyer-dev
Copy link
Collaborator

image

Pull Request Summary

Dockerfile Changes

Stage 1: Compile and Build Angular Codebase (Builder Stage)

  • Base Image:
    Uses node:20-alpine (unchanged).

  • Chrome Installation for Protractor Tests:
    The commands to install Chrome (via wget, apt-key, and apt-get) are commented out.

  • Build Dependencies:
    Installs Python, make, and g++ using apk add --no-cache.

  • Dependency Cleanup:
    Uses a sed command to remove phantomjs-prebuilt from package.json before installing dependencies.

  • Asset Preparation:
    Creates a directory for USWDS images (./src/assets/uswds/img/) and copies image assets from node_modules/@uswds/uswds/dist/img/.

  • Build Commands:
    Runs yarn run build-${environment} followed by yarn run build-dev.

Stage 2: Serve App with Nginx

  • Base Image:
    Uses nginx:1.27.2-alpine.

  • Working Directory:
    Sets working directory to /usr/share/nginx/html.

  • Static Assets:
    Removes default nginx static assets and copies the build output along with a custom nginx.conf from the builder stage.

  • Port Exposure:
    Exposes port 80.

angular.json Changes

  • Script Inclusion and Order:
    • Adjusted the order and structure of USWDS script inclusions.
    • Now includes uswds-init.min.js directly as a string and wraps uswds.min.js within an object, changing how these scripts are injected into the build.

package.json Changes

  • Dependency Updates:
    • Updated @types/node version from ^20.2.5 to ^22.10.1.
    • Minor adjustments and formatting changes to other dependencies.

src/app/app.module.ts Changes

  • Module and Import Cleanup:

    • Removed duplicate imports (e.g., TokenInterceptor and duplicate instances of NgxGoogleAnalyticsModule).
    • Consolidated module imports such as SolicitationModule and AnalyticsModule to ensure they are imported only once.
    • Tidied up LoggerModule configuration by removing redundant entries.

  • Service and Component Organization:

    • Reorganized and cleaned up service and component imports for better maintainability.

src/app/home/home.module.ts Changes

  • Expanded Module Dependencies:

    • Added Angular modules like HttpClientModule, FormsModule, ReactiveFormsModule, and RouterModule.
    • Integrated various PrimeNG modules (e.g., Button, Dropdown, Calendar, Table, Tooltip, Dialog, Checkbox, ScrollTop) to enhance UI components.
    • Introduced third-party modules such as:
      • NgxGoogleAnalyticsModule (with forRoot configuration),
      • NgxGoogleAnalyticsRouterModule, and
      • QuillEditorModule.

  • Module Reorganization:

    • Reordered declarations, providers, and exports for a cleaner module structure.

src/app/home/home/home.component.html Changes

  • Template Overhaul:

    • Hero Section Redesign:
      Updated the hero section with new classes (e.g., srt-home-head, srt-head-display, srt-head-text) and refreshed text content.

    • Tile Grid Layout Revamp:
      Redesigned the grid layout to include updated tiles/quick links for:

      • Manage/Review Solicitations (with a reset analysis button),
      • View Analytics,
      • Get Additional Resources,
      • Contact Us,
      • FAQ, and
      • Administration.

    • Navigation and Interaction:
      Updated routerLink directives and click handlers (e.g., onClickTiles) to support improved UI interactions.

src/app/home/home/home.component.scss Changes

  • Style Overhaul:

    • Responsive Grid Layout:
      Introduced a new responsive grid system for homepage tiles.

    • Hero Section Styling:
      Adjusted background image, text alignment, and responsiveness of the hero section.

    • Card and Tile Styling:

      • Added styles for card wrappers, hover effects, and consistent tile sizes.
      • Introduced new utility classes (e.g., srt-head-position, srt-head-padding, srt-icon-padding) to support the updated design.

    • Quick Links Container:
      Added styles to properly display the quick links container with updated typography and spacing.

src/app/home/home/home.component.ts Changes

  • Component Logic Enhancements:
    • Added new event handlers (e.g., onClickTiles, resetAnalysis) to manage interactions with the updated UI.
    • Introduced new state properties and logic to handle file uploads and analysis states.

src/app/services/file-upload.service.ts Changes

  • New Service Implementation:

    • Document Analysis:

      • Provides an analyzeDocuments method that sends a POST request to the backend for document analysis.
      • Uses RxJS’s firstValueFrom to convert the observable to a promise.

    • File Type Validation:

      • Implements a validateFileType method to check if a file is among the allowed types (PDF, DOC, DOCX, TXT).

This pull request brings significant updates to the build process, cleans up module imports and configurations, revamps the homepage UI/UX, and introduces a new service for document analysis.

@collinschreyer-dev
feat(manual-check): add new Home page UI, file-upload service, and Do…
4463b9a 
…ckerfile

- Implemented a new file upload interface in HomeComponent for uploading solicitation files.
- Added file-upload.service.ts (moved to src/app/services/) to handle text extraction (PDF, DOC, DOCX, TXT) and compliance analysis.
- Integrated PDF.js (with pdf.worker.min.js) and Mammoth for file processing.
- Updated Home module and app.module.ts to register the new service.
- Adjusted package.json and angular.json for new dependencies and asset configurations.
- Added Dockerfile for containerized deployment.
@collinschreyer-dev
additional css changes to home page
47558c0
@gsa-bri
Copy link
Collaborator

gsa-bri commented Feb 7, 2025 via email

BuckinghamAJ
BuckinghamAJ reviewed Feb 10, 2025
View reviewed changes
Copy link
Collaborator

@BuckinghamAJ BuckinghamAJ left a comment

Choose a reason for hiding this comment

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

Overall, I think it looks good. Had a few questions but that is it.

Dockerfile Outdated
RUN yarn run build-${environment}
RUN mkdir -p ./src/assets/uswds/img/
RUN cp -r node_modules/@uswds/uswds/dist/img/* ./src/assets/uswds/img/
RUN yarn run build-dev
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not hard code the build then all the environments will get the same build environment configuration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for catching this and the explanation!

Dockerfile Outdated
RUN yarn install

# Add the source code to app
COPY . /app/

# Generate the build of the application
RUN yarn run build-${environment}
RUN mkdir -p ./src/assets/uswds/img/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ran into some issues with some of the uswds assets when in development on this feature. I'm going to see what comes up when I remove this and test it out again. this should be handled during the build process

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good. Usually, with angular you can point to the uswds node_modules folder to access that info.

src/app/app.module.ts Outdated
import { AdminHeaderComponent } from './admin-header/admin-header.component';
import { TestModalComponent } from './test-modal/test-modal.component';
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

old code when building, needs to be removed

src/app/app.module.ts
ChartModule,

// Third Party Modules
LoggerModule.forRoot({level: NgxLoggerLevel.DEBUG}),
NgxGoogleAnalyticsModule.forRoot('G-RZRRP7Q0BH'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at this, we probably could add this to the environment configuration file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For clarification, I meant the 'G-RZRRP7Q0BH' code for the Google Analytics

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for clarifying!

src/app/home/home.module.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why add all these new modules here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This all had to do with getting the art-api to work on the home page. Now that the ART modal is working, I'll review and just keep what's necessary

Copy link
Collaborator

Choose a reason for hiding this comment

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

Very strange to me that the app.module and home.module need all of these imports. Seems redundant if that is required of angular.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll confirm this and let you know what i find

src/app/home/home/home.component.ts Outdated
/**
* Handles the scroll event to create an infinite scroll effect.
*/
private onInfiniteScroll(): void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we want infinite scroll?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No! Thank you

src/app/home/home/home.component.ts



private prepareSolicitationData() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function packages the results with necessary metadata into a structured format. It's for navigation between screens. This was developed when we discussed having a button that would take them to a detail page of the manually reviewed solicitation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we sort of decided to not have that detail page because it would be rather empty. I like the thought behind it, so maybe we should keep it if we can find a way to provide a useful additional page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, you're right about putting the detail page on hold for now. It'll be a key feature once we get the new model up and running, but I'm thinking we should just remove it for now since we're going to restructure everything anyway

src/app/home/home/home.component.ts Outdated
}

resetAnalysis(): void {
console.log('UI: Resetting analysis state');
Copy link
Collaborator

Choose a reason for hiding this comment

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

indents seem off

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, fixed!

src/app/home/home/home.component.ts
if (!this.fileUploadService.validateFileType(file)) {
throw new Error('Invalid file type');
}
const text = await this.extractText(file);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good error catching here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BuckinghamAJ BuckinghamAJ Awaiting requested review from BuckinghamAJ

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@collinschreyer-dev @gsa-bri @BuckinghamAJ