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

fix(form): support inline fields group label for required attribute #2711

Merged
merged 8 commits into from
Mar 2, 2023

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Feb 22, 2023

Description

A required Inline fields label does not get an asterisk and does not get a state color

To fix the state color i had to make use of the :has() class selector. This isnt supported by all browsers, so in those browsers this feature wont work.

I also fixed positioning of fields when an empty label is used

Testcase

Remove CSS to see issue
https://jsfiddle.net/lubber/6po2btaw/23/

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews labels Feb 22, 2023
@lubber-de lubber-de added this to the 2.9.3 milestone Feb 22, 2023
@lubber-de lubber-de changed the title fix(form): check required/disabled attribute also on field groups fix(form): support inline field group label for required attribute Feb 27, 2023
@lubber-de lubber-de changed the title fix(form): support inline field group label for required attribute fix(form): support inline fields group label for required attribute Mar 2, 2023
@lubber-de lubber-de merged commit e275dcb into fomantic:develop Mar 2, 2023
@lubber-de lubber-de deleted the fieldgroupRequiredCheck branch March 2, 2023 16:56
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Mar 2, 2023
@mvorisek
Copy link
Contributor

@lubber-de please see https://jsfiddle.net/h9ta2wdj/ repro and https://dev.atk4.org/demos/collection/tablefilter.php example usage (click column header arrow to open column filter)

In the repro, you can see extra vertical space is added - is that indented?

And please also notice the "required" asterisk is not shown either - is that indented too?

@lubber-de
Copy link
Member Author

TLDR;
Fixed by #3060

In the repro, you can see extra vertical space is added - is that indented?

(Basically) Yes, it was, because it makes sure the (empty) label keeps its height when used inside a "fields" group to still align with other fields in the same line.
image
(play with the example from the PRs original post)

And please also notice the "required" asterisk is not shown either - is that indented too?

(Basically) Yes, as, when used inside a field group (like the screenshot above), only the first field usually gets the asterisk as all following fields in the same line belong to the same label (logical wise)

Now, in your example, you used a single field and left the label empty which was not covert by this merged PR.

To fix this in the meantime without the changes in #3060, just add one space character to your "empty" label, which will then add the asterisk.
See https://jsfiddle.net/lubber/q2usjd5a/4/

lubber-de added a commit that referenced this pull request Jul 6, 2024
Followup of #2711

Whenever an empty label was used for a required single field or non-grouped, non-inline fields group, the label was still receiving an extra space to create a default height and did not display the asterisk. The intention of the space character was for empty non required fields to match same height when used inside a fields row.

If one does not want a label at all and does not want to make this field required, you would not render the label at all and wont get additional space then
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants