-
Notifications
You must be signed in to change notification settings - Fork 1
Application form layout #1000
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.
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
base: 4.x
Are you sure you want to change the base?
Application form layout #1000
Conversation
…uld not render in the parent site slot.
|
I moved this out of the draft state in order to get the documentation pages built. This is not ready for review, but the pages can be viewed at:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds application form layout components and styles for handling complex form structures within an application interface. The changes include responsive flexbox utilities, new component styles for pagination, menu navigation, and form filters, plus updated brand bar styling and corresponding Storybook showcases.
- Adds comprehensive flexbox utility classes for responsive layout management
- Introduces new component styles for pager, menu, and form-filters components
- Creates Storybook showcase stories demonstrating application form layouts with directory listings
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scss/layout/_flexbox.scss | Adds flexbox utility classes for gaps, display, wrapping, and flex values 1-12 |
| src/scss/components/pager.scss | New pagination component styles with responsive and theming support |
| src/scss/components/menu.scss | New menu component styles for both vertical and horizontal layouts |
| src/scss/components/form-filters.scss | Comprehensive form filter styles for search and advanced filtering interfaces |
| src/scss/components/brand-bar.scss | Updates font-weight reference and removes margin-left styling |
| src/scss/abstracts/_utility-classes.scss | Adds small padding utility class |
| src/scss/abstracts/_layout-main.scss | Fixes formatting by removing extra semicolons |
| src/components/showcase/ApplicationFormLayout.stories.js | New Storybook story showcasing application form layout |
| src/components/showcase/ApplicationFormDirectory.stories.js | New Storybook story showcasing directory listing with filters |
| src/components/brand-bar/BrandBar.vue | Updates import formatting and order |
| src/components/brand-bar/BrandBar.stories.js | Refactors story structure to use modern Storybook format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @include utilities.breakpoint(sm) { | ||
| column-gap: var(--space-md-width-gutter); | ||
| } |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation between .flex--gap__sm (4 spaces) and .flex--gap__md (2 spaces). Both should use the same indentation level for consistency.
| @include utilities.breakpoint(sm) { | |
| column-gap: var(--space-md-width-gutter); | |
| } | |
| @include utilities.breakpoint(sm) { | |
| column-gap: var(--space-md-width-gutter); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot. I think this should actually be the other way around with a 2 space indent.
| line-height: 1.4; | ||
| } | ||
|
|
||
| >li:hover>a:after { |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing spaces around the '>' child combinator. Should be '> li:hover > a:after' for better readability and consistency with CSS formatting standards.
| >li:hover>a:after { | |
| > li:hover > a:after { |
|
|
||
| a { | ||
| padding: 1.35rem 1.25rem; | ||
| // color: var(--brand-secondary); |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code should be removed rather than left in the codebase. If this styling might be needed later, consider using a CSS custom property or documenting the reasoning.
| // color: var(--brand-secondary); |
| <input type="text" id="last" placeholder="Last"> | ||
| </div> | ||
| <div class="form-item flex--1"> | ||
| <label for="suffix">Middle</label> |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label text 'Middle' is incorrect for a suffix field. It should be 'Suffix' to match the field purpose and placeholder text.
| <label for="suffix">Middle</label> | |
| <label for="suffix">Suffix</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot I think the label should be changed to match the text, actually.
| <div class="form-actions form-wrapper" id="edit-actions"> | ||
| <input type="submit" id="edit-submit" value="Search" class="button form-submit"> | ||
| <!-- input type="submit" id="edit-reset" name="reset" value="Reset" class="button form-submit">--> |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out HTML code should be removed from the template. If a reset button might be needed later, implement it as a conditional feature or remove the comment entirely.
| <!-- input type="submit" id="edit-reset" name="reset" value="Reset" class="button form-submit">--> |
Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com>
Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com>
…ariants (#1008) * Initial plan * Add menu component with horizontal and vertical variants Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Consolidate menu data into single menuItems array Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Remove theme prop from Menu component Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Update Menu stories to use new Storybook syntax Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Refactor menu to use PR #1000 styles with icon/arrow support Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Remove dark background story variants Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Implement recursive menu structure with MenuItems component Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Remove list markers and add icon spacing for nested menu items Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Some adjustments to get things looking more like existing menus. * Add Application Form Layout showcase stories Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Add missing pager and form-filters SCSS files from PR #1000 Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Pulled in flex styles from #1000. Adjusted showcase components to match updated form styles. * Remove Drupal-specific classes and attributes from showcase stories Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> * Cleaning up and refactoring. * Cleaned up things we are not ready for or artifacts from copilot. * Added fa-li wrapper and padding adjustments, switched from i to span tag and added presentation role. * Added container query for horizontal variant. * Moved variable inside container query. * More cleanup, simplification of CSS rules. * Added closing bracket. * Moved towards a visual pattern for vertical menus to indicate state (active, focus, hover) and nesting. * Refactored to make MenuItem vue component singular. * Added an extra icon to demonstrate better nested menus with items with both icons and no icons. Commented out some recent updates. * Cleaned up nested link highlighting. * Added suggestions for design changes. * Cleaned up recent changes. * Adjusted font-weight. * Added aria-label to menu component and stories. Active state is the same color at all nesting levels. Adjusted transitions for links to expand center out. Cleanup. * More cleanup and refinement. Added layout-container as an inline-size container. * Adjusting showcase example. Accessibility updates. * Moved application stories. Added comments and a little CSS cleanup. Set font-size for menus to avoid inconsistencies based on ul font-sizing. * Updated aria-labels based on outcome of SiteNow menu updates. * Same as last. * Update src/scss/components/menu.scss Co-authored-by: cory-skeers <49074566+cory-skeers@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pyrello <709009+pyrello@users.noreply.github.com> Co-authored-by: Sean Adams-Hiett <sean-adams-hiett@uiowa.edu> Co-authored-by: Benjamin Speare <benjamin-speare@uiowa.edu> Co-authored-by: cory-skeers <49074566+cory-skeers@users.noreply.github.com>
No description provided.