Paths

Table of Contentst

Style to add asterisk to mandatory fields in add-forge forms
ClosedPublic
Actions

Authored by jayeshv on Mar 22 2022, 11:01 AM.

Details

Reviewers

anlambert
ardumont

Group Reviewers

Reviewers

Commits

rDWAPPSfe39a1f44f6b: Style to add asterisk to mandatory fields in HTML forms

Summary

Added a separate CSS file for add-forge views

Diff Detail

Repository

rDWAPPS Web applications

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jayeshv created this revision.Mar 22 2022, 11:01 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptMar 22 2022, 11:01 AM

Build is green

Patch application report for D7405 (id=26751)

Rebasing onto 60666aa2d1...

Current branch diff-target is up to date.

Changes applied before test

commit 77517069d5514c7d6b8acbb6afbe3fd63994ccde
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Mar 22 10:55:24 2022 +0100

    Style to add asterisk to mandatory fields in add-forge forms
    
    Added a separate CSS file for add-forge views

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1524/ for more details.

Harbormaster completed remote builds in B27646: Diff 26751.Mar 22 2022, 11:21 AM

jayeshv requested review of this revision.Mar 22 2022, 11:21 AM

ardumont added a subscriber: ardumont.Mar 22 2022, 11:26 AM

ardumont added inline comments.

swh/web/templates/add_forge_now/request-dashboard.html
107	is the comment mandatory?

jayeshv added inline comments.Mar 22 2022, 11:33 AM

swh/web/templates/add_forge_now/request-dashboard.html
107	As of now it is made mandatory in the client. We don't have the final decision about this yet. The consensus seems to be to keep it mandatory for now and change later as per the feedback.

anlambert requested changes to this revision.Mar 22 2022, 11:33 AM

anlambert added a subscriber: anlambert.

anlambert added inline comments.

assets/src/bundles/add_forge/add-forge.css
8–11 ↗	(On Diff #26751)	You should put that new rule in the `webapp.css` file instead as it could be reused in other forms not related to add forge now. Also, you should prefix the class name by `swh-` to indicate this is a custom rule we wrote.
swh/web/templates/add_forge_now/request-dashboard.html
107	It should not.

This revision now requires changes to proceed.Mar 22 2022, 11:33 AM

ardumont added inline comments.Mar 22 2022, 11:50 AM

assets/src/bundles/add_forge/add-forge.css
11 ↗	(On Diff #26751)	don't we already have this somewhere else?

anlambert added inline comments.Mar 22 2022, 11:50 AM

swh/web/templates/add_forge_now/request-dashboard.html
107	Ignore my last comment, I confused with the create request form.

jayeshv added inline comments.Mar 22 2022, 11:53 AM

assets/src/bundles/add_forge/add-forge.css
11 ↗	(On Diff #26751)	I did a check in other forms but didn't find any. maybe @anlambert can answer?

Addressing review comments

Build was aborted

Patch application report for D7405 (id=26762)

Rebasing onto ac5e79103c...

First, rewinding head to replay your work on top of it...
Applying: Style to add asterisk to mandatory fields in HTML forms

Changes applied before test

commit d7fbf7bae4512425196a3c69bedb43359ead4c11
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Mar 22 11:56:30 2022 +0100

    Style to add asterisk to mandatory fields in HTML forms
    
    New CSS class is added to use with mandatory form fields
    Added this style in create and update views of add-forge-now

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1527/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1527/console

Harbormaster failed remote builds in B27683: Diff 26762!Mar 22 2022, 12:33 PM

Build is green

Patch application report for D7405 (id=26762)

Rebasing onto ac5e79103c...

First, rewinding head to replay your work on top of it...
Applying: Style to add asterisk to mandatory fields in HTML forms

Changes applied before test

commit 8f1d6853f3be7f6829b0b362d7146bc025aa7a3b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Mar 22 11:56:30 2022 +0100

    Style to add asterisk to mandatory fields in HTML forms
    
    New CSS class is added to use with mandatory form fields
    Added this style in create and update views of add-forge-now

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1529/ for more details.

Harbormaster completed remote builds in B27683: Diff 26762.Mar 22 2022, 2:44 PM

jayeshv marked 2 inline comments as done.Mar 22 2022, 2:47 PM

Looks good to me.

assets/src/bundles/add_forge/add-forge.css
11 ↗	(On Diff #26751)	For some forms like Save Code Now, we use bootstrap form validation features. It allows to highlight valid/invalid form elements with colors (green/red) and icons to indicate which actions the user has to do to be able to submit the form.

This revision is now accepted and ready to land.Mar 22 2022, 3:03 PM

Rebase

ardumont accepted this revision.Mar 22 2022, 3:41 PM

This revision was landed with ongoing or failed builds.Mar 22 2022, 3:53 PM

Closed by commit rDWAPPSfe39a1f44f6b: Style to add asterisk to mandatory fields in HTML forms (authored by jayeshv). · Explain Why

This revision was automatically updated to reflect the committed changes.

jayeshv added a commit: rDWAPPSfe39a1f44f6b: Style to add asterisk to mandatory fields in HTML forms.

Build has FAILED

Patch application report for D7405 (id=26771)

Rebasing onto ac5e79103c...

Current branch diff-target is up to date.

Changes applied before test

commit fe39a1f44f6b99d4c92948c67ae34f8e5e3417f9
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Mar 22 11:56:30 2022 +0100

    Style to add asterisk to mandatory fields in HTML forms
    
    New CSS class is added to use with mandatory form fields
    Added this style in create and update views of add-forge-now

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1535/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1535/console

Harbormaster failed remote builds in B27692: Diff 26771!Mar 22 2022, 3:54 PM