Skip to content

Clean up all major issues in existing forms.#1166

Open
mbusch3 wants to merge 22 commits into
ucfopen:form-ui-changesfrom
mbusch3:delete-element-bug
Open

Clean up all major issues in existing forms.#1166
mbusch3 wants to merge 22 commits into
ucfopen:form-ui-changesfrom
mbusch3:delete-element-bug

Conversation

@mbusch3
Copy link
Copy Markdown
Contributor

@mbusch3 mbusch3 commented Apr 28, 2026

It's totally cool. I only changed 98 files this time... I am so, so sorry.

Cleaned up some unused files...

Deleted:

  • assets/css/ContentPreview.css (old and unused)
  • assets/css/app.css (old and unused)
  • assets/css/header.css (old and unused)
  • assets/js/Components/Forms/AltText.js (renamed to AltTextForm.js)
  • assets/js/Components/Forms/AnchorText.js (renamed to AnchorTextForm.js)
  • assets/js/Components/Forms/LinkForm.js (old form only for phpAlly)
  • assets/js/Components/Forms/QuoteForm.js (only use the Blockquote one)
  • assets/js/Components/Forms/TemplateReviewOnlyForm.js (no longer used as a template)
  • assets/js/Components/ReviewFilesPage.css (reworked and included in theme)
  • assets/js/Components/Widgets/FileInformation.css (replaced in the FileForm.js with a class)
  • assets/js/Components/Widgets/FileStatus.js (replaced in the FileForm.js)
  • assets/js/Components/Widgets/FileStatus.css

Changed the Forms' Update Process

  • assets/js/Components/FixIssuesPage.js
  • assets/js/Components/FixIssuesContentPreview.js
  • assets/js/Components/HtmlPreview.js
  • assets/js/Components/UfixitWidget.js

Most forms still just rewrite a tiny bit of HTML without any major changes. When this happens, they put the new HTML in the issue.newHtml attribute and call the handleActiveIssue function. Up in the fixIssuesPage.js component, that function replaces the original element with the new one. This produces an update to the tempActiveContentItem, which is (1) ready to be saved, with all the new HTML exactly as it needs to be, and (2) sent to the htmlPreview.js component, where extra code is added just for UDOIT, like the class that shows the red outline or the Alt Text Preview widget.

Additionally, when forms also change the overall page (like the SelectValidIdForm.js), the new contentItem is passed along with the new issue in the handleActiveIssue function. This has removed the need for a separate handleActiveContentItem function and handleContentItemSave function in the FixIssuePage.js.

When the "Save" button is clicked, it just instantly saves the tempActiveContentItem which is already prepared.

Major Reworks of Certain Forms

  • assets/js/Components/Forms/ContrastForm.js
  • assets/js/Components/Forms/FileForm.js
  • assets/js/Components/Forms/ListForm.js
  • assets/js/Components/Forms/SelectValidIdForm.js

Rebuilt major sections of all of the student-built forms to make them match the rest of the app in style and the new saving flow. Wish I'd cleaned up the saving flow BEFORE they built them, but future forms really should be fairly easy to build out now.

Handle "Delete Element" Without Crashing

  • assets/js/Services/Settings.js
  • assets/js/Components/FixIssuesPage.js
  • assets/js/Components/HtmlPreview.js
  • assets/js/Components/UfixitWidget.js
  • assets/js/Components/Forms/AnchorTextForm.js
  • assets/js/Components/Forms/HeadingEmptyForm.js
  • assets/js/Components/Forms/TableCaptionForm.js

On forms where one of the valid options is to delete the problematic element, special care must be taken to make sure that the preview doesn't crash. Now, when a form has selected that option, the preview DOES delete the element but does NOT launch the "Accessibility Barrier Not Found" popup or lose focus on the preview screen.

Changed Where User Preference Classes are Placed

  • assets/js/Components/App.js
  • assets/css/udoit4-theme.css

Before, all of the classes that render the app differently based on preferences (font type and size, dark mode, etc) were added to the #app-container element. That worked well EXCEPT when we want to apply these styles to TinyMCE popups and other dialogs which are added programmatically. App.js now has an addSettingsClasses function, which applies the classes to the <body> element, where they can be inherited by <dialog>s.

Full Screen Option

  • assets/js/Components/Header.css
  • assets/js/Components/Header.js

Completely reworked the header to add a full-screen option. It was surprisingly difficult to get everything to align properly and compute when to show the hamburger menu instead of the link names.

Dynamic Colors for Reports Screen

  • assets/js/Components/Reports/ResolutionsReport.css
  • assets/js/Components/Reports/ResolutionsReport.js

Made accessible colors for the reports chart in both light and dark mode.

Comprehensive Design Audit

  • All the files...

Worked with Tory over three different days to do a final version of all flows and pages in both light and dark mode across all screen sizes. A whole lot of small CSS and class changes that affected the entire app.

@mbusch3 mbusch3 requested a review from dmols April 28, 2026 18:56
@mbusch3 mbusch3 changed the base branch from dev to form-ui-changes April 28, 2026 19:17
@Ishfaq-code
Copy link
Copy Markdown
Contributor

The contrast form (ContrastForm.js) seems to target pages which have relevant/good contrast. This maybe a frontend calculation issue or something to do with how backend is categorizing stuff.

Screen.Recording.2026-05-07.at.12.53.32.PM.mov

@Ishfaq-code
Copy link
Copy Markdown
Contributor

Seems like if there are no changes to be saved, you can actually spam the save button and create an endless chain of alerts. Some sort of cooldown could be added on the frontend to prevent this.

Screen.Recording.2026-05-07.at.1.00.13.PM.mov

@Ishfaq-code
Copy link
Copy Markdown
Contributor

Close form after deleting something (such as from AnchorTextForm.js) may be more intuitive as user is not able to edit the form anymore. Seems redundant with the error message

@Ishfaq-code
Copy link
Copy Markdown
Contributor

The focus seems to acting a bit weird on the SelectValidIDForm.js, specifically if something is getting unselected the focus effect does not disappear. I do remember fixing this in my PR a while back but might have been removed during merge?

Screen.Recording.2026-05-08.at.11.22.10.AM.mov

@Ishfaq-code
Copy link
Copy Markdown
Contributor

The FileForm.js is broken in this branch and dev. This is due to one of the packages not being installed when updating PHP and Symfony. This issue is addressed by #1180 and should be fixed once merged.

@Ishfaq-code
Copy link
Copy Markdown
Contributor

The List Form seems to have some weird behaviour with saving (I don't know if it is intentional). On a successful save the modal jumps to the next issue (or close the modal) rather than showing the fixed state of the issue. This may due to how list forms are grouped.

@Ishfaq-code
Copy link
Copy Markdown
Contributor

The List form only marks an element as a potential barrier if it has 1 white space after the list indicator (numbers or dashes).

For example

1.With no whitespace after the "." 

2. With 1 whitespace after the "." 

3.  With 2 whitespaces after the "."

 

-With no whitespace after the "." 

- With 1 whitespace after the "." 

-  With 1 whitespace after the "."

Only the middle element is marked as a list error in both of these cases

@Ishfaq-code
Copy link
Copy Markdown
Contributor

Text spacing slider which is found in the settings page seems to be broken. It does not create spacing between text locally or in the database.

@Ishfaq-code
Copy link
Copy Markdown
Contributor

Some issues with the Spanish Translations. May be prevalent in dev as well.
Screenshot 2026-05-08 at 3 07 32 PM
Screenshot 2026-05-08 at 3 09 20 PM
Screenshot 2026-05-08 at 3 10 08 PM

@Ishfaq-code
Copy link
Copy Markdown
Contributor

The FileForm.js is broken in this branch and dev. This is due to one of the packages not being installed when updating PHP and Symfony. This issue is addressed by #1180 and should be fixed once merged.

I manually installed Symfony/Mime on this branch to test the files and they seem to be working as intended. Should be operational once we merge the branch containing Symfony/Mime into dev and pull dev into here.

Copy link
Copy Markdown
Contributor

@Ishfaq-code Ishfaq-code left a comment

Choose a reason for hiding this comment

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

Functionality wise the forms look good and useable. Couple of tiny bugs that mess with the UI/nitpicks but nothing that makes the forms unusable in anyway. Should be fairly easy fixes and then good to merge.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants