usnistgov / oar-pdr

The NIST Open Access to Research (OAR) Public Data Repository (PDR) system software
11 stars 10 forks source link

LPS Restructuring: Introducing Section Components #196

Closed RayPlante closed 2 years ago

RayPlante commented 2 years ago

This PR represents the intended last step in the restructuring of the Landing Page Service code to improve maintainability. The first major step (#120) was to organize the landing page into a frame (with header and footer), the tools menu, and the man landing page content (see cartoons as part of PR#120). The next step (#185) was to extract datacata and file-tree functionality out of the LandingComponent and its children by establishing clear communication channels with an encapsulated datacart. In this final step, we break the landing page into decomposable parts organized around the logical document sections

The change provided by this PR is summarize in the following cartoon: IntoSections

This restructuring is characterized by the following changes:

chuanlin2018 commented 2 years ago

Beautiful structure! I like it. Found couple of minor issues so far:

  1. The "Go TO" menu doesn't seem to be working.
  2. The "Metadata" item in "Go To" menu expands the metadata the first time. If I manually collapse metadata, The "Metadata" button no longer expands it.

Still exploring your code...

chuanlin2018 commented 2 years ago

Found one more problem in edit mode. So total three problems. I am not sure if I should check in my code because I made changes in multiple components. Let me explain here:

Issue #1: The "Go To" menu didn't scroll the page at all in my local version. It did collapse the metadata. For some reason the following script didn't work: this.router.navigate(['/od/id/', this.reqId], { fragment: sectionId }); I have to use nativeElement.scrollIntoView to scroll to a desire section.

Issue#2: The Metadata button stopped working if user collapses the metadata manually. In this case, there are two things to fix: 1) When user manually collapses metadata, ngOnChange no longer get triggered when user click on Metadata button. I have to use setter/getter to pass the "ShowMetadata" value to Metadata component instead of @Input. 2) Even "ShowMetadata" value get passed to Metadata component correctly, the metadata only gets expanded for the first time. If user manually collapse it, it won't be expanded by the Metadata button again. The solution is to change p-fieldset attribute from [collapsed]="collapsed" to [(collapsed)]="collapsed".

Issue #3: In edit mode, the control buttons (Save, Cancel, ...) in all popup windows do not display correctly. The solution is to add ButtonModule to each module.ts and add following in-line style to p-toolbar: [style]="{'background':'none','border-width':'0'}".

I can show you my changes to see if you have any better solution.

chuanlin2018 commented 2 years ago

Just checked in my code to branch restruct/sections3-chuan. Following are modified files:

modified:   src/app/landing/author/author-popup/author-popup.component.css
modified:   src/app/landing/author/author-popup/author-popup.component.html
modified:   src/app/landing/author/author.module.ts
modified:   src/app/landing/contact/contact-popup/contact-popup.component.html
modified:   src/app/landing/contact/contact.module.ts
modified:   src/app/landing/description/description-popup/description-popup.component.html
modified:   src/app/landing/description/description.module.ts
modified:   src/app/landing/landingbody.component.html
modified:   src/app/landing/landingbody.component.ts
modified:   src/app/landing/landingpage.component.html
modified:   src/app/landing/landingpage.component.ts
modified:   src/app/landing/metadata/metadata.component.ts
modified:   src/app/landing/metadata/metadata.module.ts
modified:   src/app/landing/sections/resourcedata.component.html
modified:   src/app/landing/sections/resourcedata.component.ts
modified:   src/app/landing/sections/resourcemetadata.component.html
modified:   src/app/landing/sections/resourcemetadata.component.ts
modified:   src/app/landing/topic/topic-popup/search-topics.component.html
modified:   src/app/landing/topic/topic.module.ts
chuanlin2018 commented 2 years ago

Just realized that I didn't checkin the changes. Now checked in.

RayPlante commented 2 years ago

@chuanlin2018 I had to tweaked a few things:

Can you give this a test?

chuanlin2018 commented 2 years ago

Will do.

RayPlante commented 2 years ago

If you think it's ready, go ahead and approve the PR and merge it. Thanks!