Page MenuHomeSoftware Heritage
Differential D7405

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
Branch
forge-styles
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27683
Build 43319: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 43318: arc lint + arc unit

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
swh-public-ci added a comment.Mar 22 2022, 11:20 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?

jayeshv updated this revision to Diff 26762.Mar 22 2022, 12:03 PM

Addressing review comments

swh-public-ci added a comment.Mar 22 2022, 12:33 PM

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
swh-public-ci added a comment.Mar 22 2022, 2:44 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
anlambert accepted this revision.Mar 22 2022, 3:03 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
jayeshv updated this revision to Diff 26771.Mar 22 2022, 3:35 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.
swh-public-ci added a comment.Mar 22 2022, 3:54 PM

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

Revision Contents
Changeset List

PathSize
assets/
src/
bundles/
webapp/
5 lines
swh/
web/
templates/
add_forge_now/
8 lines
6 lines

Diff 26762

View Options

assets/src/bundles/webapp/webapp.css

Loading...
View Options

swh/web/templates/add_forge_now/create-request.html

Loading...
View Options

swh/web/templates/add_forge_now/request-dashboard.html

Loading...
About · Developer information · Legal