Skip to content
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

Fixed input field misalignments #15552

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Conversation

Godmartinz
Copy link
Collaborator

Description

inputs were being resized to make room for the required class if required. By changing the required class to a pseudo class (input:required, select:required, etc) the required css highlights are applied inside of the input.

BEFORE:
Create Asset:
image

Create Accessory:
image

AFTER:
Create Asset:
image

Create Accessory:
image

Fixes #[sc-26893]

Type of change

Please delete options that are not relevant.

  • [x ] 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 not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • PHP version:
  • MySQL version
  • Webserver version
  • OS version

Checklist:

Copy link

what-the-diff bot commented Sep 24, 2024

PR Summary

  • Update of Required Field Selector
    Changed the way the app identifies required fields. It now seeks out any required input or selection elements, enhancing identification speed and accuracy.

  • Adjustment to Border Thickness of Required Fields
    The border surrounding required input fields are now slightly thinner. This offers a less obtrusive, yet equally clear, visual cue that these fields are mandatory.

  • Introduction of Styling for Select2 Containers
    New styles have been introduced for Select2 containers linked to required selection elements. This improves user experience by clearly indicating selections that need to be made.

@snipe
Copy link
Owner

snipe commented Sep 25, 2024

Hm. I sort of liked the requiredness color being not-attached to the input item itself, but I guess I can live with this. Thanks!

@snipe snipe merged commit 21f8ac8 into snipe:develop Sep 25, 2024
9 checks passed
@snipe
Copy link
Owner

snipe commented Sep 28, 2024

I'm noticing a few places where the requiredness was lost (example: /hardware/maintenances/create). I think we need to take another look at this.

Screenshot 2024-09-28 at 1 17 29 PM

@snipe
Copy link
Owner

snipe commented Sep 28, 2024

I see what happened here. We were using the required class on the container div, not always using the required HTML element. This change breaks the visual display of requiredness because not all of our required fields are flagged as required on the input/select element, but rather in the div class that contains it. We'll have to update all of the form fields to reflect this new approach.

@Godmartinz
Copy link
Collaborator Author

Exactly, I didnt realize that was the case elsewhere til modals. I can get the rest of the app cleaned up Monday. 🙂

@snipe
Copy link
Owner

snipe commented Sep 28, 2024

I’m already working on it :) PR shortly

@snipe
Copy link
Owner

snipe commented Sep 29, 2024

Unfortunately, this change means the multi-checkout-to selector doesn't get highlighted as required anymore, since we can't make non-focusable things required. Javascript gets angry. Not sure what the solve for this is though. :(

snipe added a commit that referenced this pull request Sep 29, 2024
Related to the discussion at #15552

Signed-off-by: snipe <[email protected]>
P-Vamshi-Seshasayan pushed a commit to P-Vamshi-Seshasayan/snipe-it that referenced this pull request Nov 4, 2024
Related to the discussion at snipe#15552

Signed-off-by: snipe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants