Skip to content

Adjustments to style of select tags and plan download layout #3509

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

Conversation

johnpinto1
Copy link
Contributor

@johnpinto1 johnpinto1 commented Apr 15, 2025

Contributed by @gjacob24 of DCC DMPonline.
Code pushed by @johnpinto1 for DCC DMPonline.

Changes:

  • Replaced Selection_tag classes incorrectly set with 'form-control' to 'form-select'.
  • Adjusts the layout and displays the 'optional plan components' and 'select phase to download' side by side for multi-phase plans (cf. images below).

Before Change:
Selection_025
With Change
Selection_026

Copy link

github-actions bot commented Apr 15, 2025

1 Warning
⚠️ There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior. \n
Ignore this warning if the PR ONLY contains translation.io synced updates.

Generated by 🚫 Danger

@johnpinto1 johnpinto1 force-pushed the gjacob24-contribution-of-style-of-select-tags-and-plan-download-layout branch from 56b07ea to c8742a6 Compare April 15, 2025 10:18
@johnpinto1 johnpinto1 self-assigned this Apr 15, 2025
Copy link
Contributor

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

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

I'm liking all of the form-control to form-select updates here, and I may have found some additional needed ones as well. I've created PR #3517 which is pointed at this current PR. Please let me know what you think, thanks. :)

@gjacob24
Copy link
Contributor

I'm liking all of the form-control to form-select updates here, and I may have found some additional needed ones as well. I've created PR #3517 which is pointed at this current PR. Please let me know what you think, thanks. :)

Thanks @aaronskiba! We noticed that there may be a few more missing. So, will add to your PR next week if you haven't already covered it :)

@johnpinto1 johnpinto1 force-pushed the gjacob24-contribution-of-style-of-select-tags-and-plan-download-layout branch 2 times, most recently from c1ce784 to 1635e58 Compare May 1, 2025 09:35
John Pinto and others added 2 commits May 1, 2025 10:51
Contributed by @gjacob24 of DCC DMPonline.

The changes:
 - Replaced Selection_tag classes incorrectly set with  'form-control'
   to 'form-select'.

Co-authored-by: gjacob24 <[email protected]>
    Contributed by @gjacob24 of DCC DMPonline.
    Changes:
    - Adjusts the layout and displays the 'optional plan components' and 'select phase to download' side by side for multi-phase plans.

    Co-authored-by: gjacob24 <[email protected]>
@johnpinto1 johnpinto1 force-pushed the gjacob24-contribution-of-style-of-select-tags-and-plan-download-layout branch from 1635e58 to 36ba6aa Compare May 1, 2025 09:53
aaronskiba and others added 5 commits May 1, 2025 09:16
- This changes expand upon commit 87ddbb0
- This change improves the styling/positioning between the "Optional plan components" and "Select phase to download" headings.
  - Added "form-control" class to "Optional plan components" heading to match "Select phase to download" stylings
  - Replaced `<legend>` element with a `label_tag`+ `form-label` class to again improve consistency between the two headings.
    - Note, the added `label_tag` doesn't associate with any specific form control, so the first paramater is left empty.
…of-style-of-select-tags-and-plan-download-layout

Additional `form-control` to `form-select` changes & styling changes to "Download settings" subheadings
- Commit 72c2519 improved the alignment between the 'Optional plan components' and 'Select phase to download' headings. However, the change from <legend> to an unreferenced label_tag() hurt the accessibility.

- This change puts back the <legend> tag and uses class: 'col-form-label' for both of the aforementioned headings to maintain stylistic consistency between the two elements.
  - `.col-form-label` overrides <legend> styling to maintain consistent alignment with <label> elements.

- Accessibility snapshot of <fieldset> before this fix:
Name: ""
aria-labelledby: Not specified
aria-label: Not specified
From legend: Not specified
title: Not specified
Role: group
Invalid user entry: false

- Accessibility snapshot of <fieldset> after this fix:
Name: "Optional Plan Components"
aria-labelledby: Not specified
aria-label: Not specified
From legend: legend.col-form-label "Optional Plan Components"
title: Not specified
Role: group
Invalid user entry: false
Labeled by: legend.col-form-label
Copy link
Contributor

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

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

I've merged my approved PR regarding additional modifications to this PR (#3517).

I am approving this PR, but I've also made one final change that you may want to inspect before merging (3277370). The change is meant to serve as an accessibility fix on another commit made by me (72c2519).

@johnpinto1 johnpinto1 merged commit 4c1befb into development May 2, 2025
8 checks passed
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.

3 participants