feat: UI Extensibility #445

arumsey · 2024-10-31T14:00:30Z

Description

fix swc overlays
add top nav bar
add light/dark theming
add preact to componentize new features
fix toast usage guidance
modularize document loading
modularize import functionality
add contextual help
minor UI improvements to match Spectrum guidelines and branding

Related Issue

Fixes #446

Motivation and Context

The roadmap for the AEM Importer includes adding advancements in the Importer to reduce onboarding time and effort. The motivation for changes in this PR are to provide a framework for future feature development and ensure the code remains maintainable.

How Has This Been Tested?

Screenshots:

Existing

New

Light theme

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

- fix swc overlays - add top nav bar - add light/dark theming - add preact to componentize new features - fix toast usage guidance - modularize document loading - modularize import functionality - add contextual help - minor UI improvements to match Spectrum guidelines and branding

atopper · 2024-11-04T21:07:34Z

css/styles.css

-  grid-template-rows: 1fr; 
+body.loading {
+  display: none;
+}


Is this an initializing maybe? We may load things later at runtime and need spinners, or something.

Yes - swc is now being loaded asynchronously so we need to wait for the loading to complete to avoid seeing font/styling changes. Since swc is local the async load is fast so adding a spinner would be a distraction.

Right.
I meant could we call the class "initializing" so when we are loading something later (report, file, scripts, loading something from S3 someday, whatever), we won't get collisions with the 'loading' class.

Oh, you're worried about the loading class name? I guess I could change it to hidden since that is what we're actually doing.

We can still have a different behaviour for the loading class when it is applied to different elements. When body has a loading class we hid all the content. When a div has a loading class we could display a spinner. The behaviour of the class should be based on where it is used on not what its name is.

But, I can change the class name to hidden if you think that is more semantically correct.

import-bulk.html

js/crawl/crawl.ui.js

js/import/import.preview.js

atopper

Lots small changes in that are hard to absorb - lots of styling changes and I trust it all looks good (thanks for the screenshot). Everything I see looks good. 👍

- minor review changes

catalan-adobe · 2024-11-05T10:36:02Z

@arumsey, I've been testing the changes locally and everything so far is working as before
BUT in the import views, while the import is running the "Download Import Report" appears but then disappears when import is done.

- fix `toggleReportButton` logic

arumsey · 2024-11-12T14:46:09Z

@arumsey, I've been testing the changes locally and everything so far is working as before BUT in the import views, while the import is running the "Download Import Report" appears but then disappears when import is done.

@catalan-adobe The toggleReportButton report button logic has been fixed.

# Conflicts: # js/dist/helix-importer.js

- re-build `helix-importer`

- minor top nav fixes

arumsey · 2024-11-14T21:23:03Z

@catalan-adobe When you have a chance can you review agian please?

catalan-adobe · 2024-11-18T14:28:55Z

@arumsey, I just checked the "download report button" again and the UI issue is fixed but clicking it does not do anything. Report is not being downloaded (I checked the web console, no specific error messages there).

- fix download report action

arumsey · 2024-11-18T19:01:40Z

@arumsey, I just checked the "download report button" again and the UI issue is fixed but clicking it does not do anything. Report is not being downloaded (I checked the web console, no specific error messages there).

@catalan-adobe Shoot! The DOWNLOAD_IMPORT_REPORT_BUTTON click listener was using PreviewElements instead of PreviewButtons. The click event is now wired up correctly and a report is being generated from importStatus.

# Conflicts: # js/dist/helix-importer.js # package-lock.json

- re-build after merging from main

catalan-adobe

@atopper , download report works now! PR LGTM!

atopper

I ran it and sent a few observations to you directly. "Download" button in Bulk mode is the only issue in my opinion.

- add border to preview frame - fix Code Mirror theming

arumsey · 2024-11-20T19:46:15Z

I ran it and sent a few observations to you directly. "Download" button in Bulk mode is the only issue in my opinion.

@atopper All of your issues should be resolved now.

# Conflicts: # js/dist/helix-importer.js

- rebuild helix-importer

atopper

👍

arumsey requested review from catalan-adobe, karlpauls and atopper November 4, 2024 14:13

arumsey self-assigned this Nov 4, 2024

atopper reviewed Nov 4, 2024

View reviewed changes

import-bulk.html Show resolved Hide resolved

atopper reviewed Nov 4, 2024

View reviewed changes

js/crawl/crawl.ui.js Outdated Show resolved Hide resolved

atopper reviewed Nov 4, 2024

View reviewed changes

js/import/import.preview.js Outdated Show resolved Hide resolved

atopper previously approved these changes Nov 4, 2024

View reviewed changes

feat: UI Extensibility

7f11942

- minor review changes

arumsey dismissed atopper’s stale review via 7f11942 November 4, 2024 21:39

feat: UI Extensibility

af642f1

- fix `toggleReportButton` logic

arumsey added 3 commits November 12, 2024 09:49

Merge branch 'main' into ui-extensibility

03ef4b7

# Conflicts: # js/dist/helix-importer.js

feat: UI Extensibility

ea48118

- re-build `helix-importer`

feat: UI Extensibility

e00c34c

- minor top nav fixes

arumsey requested a review from atopper November 14, 2024 21:22

feat: UI Extensibility

a1f32d2

- fix download report action

arumsey added 3 commits November 18, 2024 14:24

Merge branch 'main' into ui-extensibility

e0b38bd

# Conflicts: # js/dist/helix-importer.js # package-lock.json

feat: UI Extensibility

988e37f

- re-build after merging from main

feat: UI Extensibility

16272c3

- re-build after merging from main

catalan-adobe previously approved these changes Nov 19, 2024

View reviewed changes

atopper previously approved these changes Nov 19, 2024

View reviewed changes

feat: UI Extensibility

6a3441a

- add border to preview frame - fix Code Mirror theming

arumsey dismissed atopper’s stale review via 6a3441a November 20, 2024 19:44

arumsey dismissed catalan-adobe’s stale review via 6a3441a November 20, 2024 19:44

arumsey added 2 commits November 20, 2024 14:47

Merge branch 'main' into ui-extensibility

301f0ab

# Conflicts: # js/dist/helix-importer.js

feat: UI Extensibility

1424205

- rebuild helix-importer

atopper approved these changes Nov 20, 2024

View reviewed changes

arumsey requested review from catalan-adobe and kptdobe November 20, 2024 21:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: UI Extensibility #445

feat: UI Extensibility #445

arumsey commented Oct 31, 2024 •

edited

Loading

atopper Nov 4, 2024

arumsey Nov 4, 2024

atopper Nov 4, 2024 •

edited

Loading

arumsey Nov 4, 2024

arumsey Nov 12, 2024

atopper left a comment

catalan-adobe commented Nov 5, 2024 •

edited

Loading

arumsey commented Nov 12, 2024

arumsey commented Nov 14, 2024

catalan-adobe commented Nov 18, 2024

arumsey commented Nov 18, 2024

catalan-adobe left a comment

atopper left a comment

arumsey commented Nov 20, 2024

atopper left a comment

feat: UI Extensibility #445

Are you sure you want to change the base?

feat: UI Extensibility #445

Conversation

arumsey commented Oct 31, 2024 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots:

Existing

New

Light theme

Types of changes

Checklist:

atopper Nov 4, 2024

Choose a reason for hiding this comment

arumsey Nov 4, 2024

Choose a reason for hiding this comment

atopper Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

arumsey Nov 4, 2024

Choose a reason for hiding this comment

arumsey Nov 12, 2024

Choose a reason for hiding this comment

atopper left a comment

Choose a reason for hiding this comment

catalan-adobe commented Nov 5, 2024 • edited Loading

arumsey commented Nov 12, 2024

arumsey commented Nov 14, 2024

catalan-adobe commented Nov 18, 2024

arumsey commented Nov 18, 2024

catalan-adobe left a comment

Choose a reason for hiding this comment

atopper left a comment

Choose a reason for hiding this comment

arumsey commented Nov 20, 2024

atopper left a comment

Choose a reason for hiding this comment

arumsey commented Oct 31, 2024 •

edited

Loading

atopper Nov 4, 2024 •

edited

Loading

catalan-adobe commented Nov 5, 2024 •

edited

Loading