[JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone #8491

flabrie · 2023-09-14T19:55:37Z

Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone, because there are missing CSS class definitions.

See JENKINS-72018.

Testing done

Yes, and adding the missing CSS classes fix the issue.

Proposed changelog entries

Hide the delete button from the only repeatable element in configuration forms when at least one element is expected. (regression in 2.344)

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The Jira issue, if it exists, is well-described.

The Jira issue, if it exists, is well-described.
Options
Successfully updated the issue's project

There was an error updating the issue's project
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see [examples](https://github.com/jenkins-infra/jenkins.io/blob/master/content/_data/changelogs/weekly.yml)). Fill in the **Proposed upgrade guidelines** section only if there are breaking changes or changes that may require extra steps from users during upgrade.

The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
Options
Successfully updated the issue's project

There was an error updating the issue's project
There is automated testing or an explanation as to why this change has no tests.

There is automated testing or an explanation as to why this change has no tests.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New public classes, fields, and methods are annotated with `@Restricted` or have `@since TODO` Javadocs, as appropriate.

New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New deprecations are annotated with `@Deprecated(since = "TODO")` or `@Deprecated(forRemoval = true, since = "TODO")`, if applicable.

New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New or substantially changed JavaScript is not defined inline and does not call `eval` to ease future introduction of Content Security Policy (CSP) directives (see [documentation](https://www.jenkins.io/doc/developer/security/csp/)).

New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
Options
Successfully updated the issue's project

There was an error updating the issue's project
For dependency updates, there are links to external changelogs and, if possible, full differentials.

For dependency updates, there are links to external changelogs and, if possible, full differentials.
Options
Successfully updated the issue's project

There was an error updating the issue's project
For new APIs and extension points, there is a link to at least one consumer.

For new APIs and extension points, there is a link to at least one consumer.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Give feedback

There are at least two (2) approvals for the pull request and no outstanding requests for change.

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.

Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Changelog entries in the pull request title and/or **Proposed changelog entries** are accurate, human-readable, and in the imperative mood.

Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Proper changelog labels are set so that the changelog can be generated automatically.

Proper changelog labels are set so that the changelog can be generated automatically.
Options
Successfully updated the issue's project

There was an error updating the issue's project
If the change needs additional upgrade steps from users, the `upgrade-guide-needed` label is set and there is a **Proposed upgrade guidelines** section in the pull request title (see [example](https://github.com/jenkinsci/jenkins/pull/4387)).

If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
Options
Successfully updated the issue's project

There was an error updating the issue's project
If it would make sense to backport the change to LTS, a Jira issue must exist, be a _Bug_ or _Improvement_, and be labeled as `lts-candidate` to be considered (see [query](https://issues.jenkins.io/issues/?filter=12146)).

If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

…ted-chunk .show-if-not-only ones. Without these CSS classes, form entries using .show-if-not-only class are not hidden when alone.

daniel-beck · 2023-09-15T10:39:05Z

Effectively reverts #6471.

Probably re-introduces the warnings motivating that PR (but for correct behavior, seems the lesser evil).

daniel-beck · 2023-09-15T11:07:56Z

Interactive test with Subversion plugin (see screenshots in Jira) passes, but autotests fail. Looks like HTMLUnit may lack support for what's a legal (and quite basic?) selector.

Unsure how to go about fixing those, perhaps JTH change to make https://github.com/jenkinsci/jenkins-test-harness/blob/3b9790f39c3106398edda1133833caf9c31532af/src/main/java/org/jvnet/hudson/test/JenkinsRule.java#L2337 more tolerant, reverting parts of jenkinsci/jenkins-test-harness#530, and applying the same change to

jenkins/test/src/test/java/jenkins/bugs/Jenkins14749Test.java

Lines 77 to 78 in c5fbbbe

    
           // Keep in sync with HudsonTestCase/JenkinsRule 
        
           return exception.getURI().contains("/yui/");

?

Ideally we'd find a way to express the same rule in a manner not resulting in an HTMLUnit warning however.

daniel-beck · 2023-09-15T15:37:03Z

Per HtmlUnit/htmlunit-cssparser#36, only seems to be treated like a reserved word in HTMLUnit's CSS parser.

Awkward that it's documented in

jenkins/core/src/main/resources/lib/form/repeatable.jelly

Line 54 in c5fbbbe

    
                   only  : it is the only chunk (automatically get first and last at the same time)

so just renaming the class might be out.

Perhaps

$ git diff war/src/
diff --git a/war/src/main/scss/form/_reorderable-list.scss b/war/src/main/scss/form/_reorderable-list.scss
index dfd0fc5c5a..c19152975d 100644
--- a/war/src/main/scss/form/_reorderable-list.scss
+++ b/war/src/main/scss/form/_reorderable-list.scss
@@ -29,7 +29,7 @@
   visibility: visible;
 }
 
-.repeated-chunk.only .show-if-not-only {
+.repeated-chunk.first.last .show-if-not-only {
   visibility: hidden;
 }
 
@@ -54,7 +54,7 @@
   visibility: visible;
 }
 
-.repeated-chunk .repeated-chunk.only .show-if-not-only {
+.repeated-chunk .repeated-chunk.first.last .show-if-not-only {
   visibility: hidden;
 }
 
@@ -79,7 +79,7 @@
   visibility: visible;
 }
 
-.repeated-chunk .repeated-chunk .repeated-chunk.only .show-if-not-only {
+.repeated-chunk .repeated-chunk .repeated-chunk.first.last .show-if-not-only {
   visibility: hidden;
 }

This makes the autotest pass at least (not manually tested).

daniel-beck

(Per previous comment)

war/src/main/scss/form/_reorderable-list.scss

Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>

daniel-beck

Works well in interactive testing with Subversion SCM modules in freestyle jobs.

Nesting of selectors looks error-prone, but this basically only restores what we had before the "cleanup", so seems tolerable.

When going from "not only" to "only", the "X" disappears well before the grey circle, but it doesn't look too broken.

timja

nice fix

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

- Add the missing definition for the .only CSS class inside of .repea…

2cae901

…ted-chunk .show-if-not-only ones. Without these CSS classes, form entries using .show-if-not-only class are not hidden when alone.

daniel-beck changed the title ~~Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone~~ [JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone Sep 15, 2023

daniel-beck added the regression-fix Pull request that fixes a regression in one of the previous Jenkins releases label Sep 15, 2023

daniel-beck requested changes Sep 15, 2023

View reviewed changes

war/src/main/scss/form/_reorderable-list.scss Outdated Show resolved Hide resolved

war/src/main/scss/form/_reorderable-list.scss Outdated Show resolved Hide resolved

war/src/main/scss/form/_reorderable-list.scss Outdated Show resolved Hide resolved

flabrie and others added 3 commits September 18, 2023 08:32

Update war/src/main/scss/form/_reorderable-list.scss

958edb0

Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>

Update war/src/main/scss/form/_reorderable-list.scss

601373e

Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>

Update war/src/main/scss/form/_reorderable-list.scss

e0bf6c3

Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>

daniel-beck added the squash-merge-me Unclean or useless commit history, should be merged only with squash-merge label Sep 18, 2023

daniel-beck self-requested a review September 18, 2023 14:28

daniel-beck approved these changes Sep 18, 2023

View reviewed changes

daniel-beck added the bug For changelog: Minor bug. Will be listed after features label Sep 18, 2023

NotMyFault requested a review from a team September 18, 2023 20:25

timja approved these changes Sep 22, 2023

View reviewed changes

comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Sep 22, 2023

MarkEWaite merged commit c0f6fbe into jenkinsci:master Sep 24, 2023
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone #8491

[JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone #8491

flabrie commented Sep 14, 2023 •

edited by daniel-beck

Submitter checklist

Maintainer checklist

daniel-beck commented Sep 15, 2023

daniel-beck commented Sep 15, 2023 •

edited

daniel-beck commented Sep 15, 2023

daniel-beck left a comment

daniel-beck left a comment

timja left a comment

[JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone #8491

[JENKINS-72018] Form entries in .repeated-chunk using .show-if-not-only CSS class are not hidden when they are alone #8491

Conversation

flabrie commented Sep 14, 2023 • edited by daniel-beck

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

daniel-beck commented Sep 15, 2023

daniel-beck commented Sep 15, 2023 • edited

daniel-beck commented Sep 15, 2023

daniel-beck left a comment

Choose a reason for hiding this comment

daniel-beck left a comment

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

flabrie commented Sep 14, 2023 •

edited by daniel-beck

daniel-beck commented Sep 15, 2023 •

edited