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

web: Fix label not clickable for checkbox and choice field in prompts #5355

Merged
merged 2 commits into from
Apr 28, 2023
Merged

web: Fix label not clickable for checkbox and choice field in prompts #5355

merged 2 commits into from
Apr 28, 2023

Conversation

macmoritz
Copy link
Contributor

@macmoritz macmoritz commented Apr 23, 2023

Details

Labels next to checkboxes and choice field in prompts are not clickable.
Therefor we need to click on the checkbox or choicebox directly which is rather small.

Changes

Fix

Wrap the checkbox/choicebox in the HTML label.

New Features

Labels next to checkboxes and choice fields are now clickable

Additional

Using this method has some advantages over the HTML for attribute:

  • No need to assign an unique id to every checkbox.
  • No need to use the extra attribute in the label.
  • Delivered page has same usage of storage (yes, for would be minor, but it is a fact)

@BeryJu
Copy link
Member

BeryJu commented Apr 24, 2023

The reason why the structure of the labels is what it is, is because of the UI library (see https://www.patternfly.org/v4/components/checkbox/html/)

We can use the fieldKey attribute as id and use that with for, and also I noticed the prompt stage is missing the actual checkbox CSS from import PFCheck from "@patternfly/patternfly/components/Check/check.css";.

@macmoritz
Copy link
Contributor Author

Would be great if we can fix this.
When I have time I will take a look at it.

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.09 ⚠️

Comparison is base (158fe2f) 92.72% compared to head (5a68962) 92.62%.

❗ Current head 5a68962 differs from pull request most recent head 8f7ff5d. Consider uploading reports for the commit 8f7ff5d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5355      +/-   ##
==========================================
- Coverage   92.72%   92.62%   -0.09%     
==========================================
  Files         506      506              
  Lines       25908    25908              
==========================================
- Hits        24020    23995      -25     
- Misses       1888     1913      +25     
Flag Coverage Δ
e2e 51.87% <ø> (-0.74%) ⬇️
integration 26.27% <ø> (-0.05%) ⬇️
unit 89.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

# Conflicts:
#	web/src/flow/stages/prompt/PromptStage.ts
@BeryJu BeryJu changed the title Fix label not clickable for checkbox and choice field in prompts web: Fix label not clickable for checkbox and choice field in prompts Apr 28, 2023
@BeryJu BeryJu merged commit 0166346 into goauthentik:main Apr 28, 2023
32 of 33 checks passed
@netlify
Copy link

netlify bot commented Apr 28, 2023

Deploy Preview for authentik ready!

Name Link
🔨 Latest commit 8f7ff5d
🔍 Latest deploy log https://app.netlify.com/sites/authentik/deploys/644b90f183ebd70008ed1054
😎 Deploy Preview https://deploy-preview-5355--authentik.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@macmoritz macmoritz deleted the checkbox-label-clickable branch May 4, 2023 06:20
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.

None yet

2 participants