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

WD-3904-credentials-schedule #12988

Merged
merged 8 commits into from
Jul 26, 2023
Merged

Conversation

abhigyanghosh30
Copy link
Contributor

@abhigyanghosh30 abhigyanghosh30 commented Jun 23, 2023

Done

  • [List of work items including drive-bys - remember to add the why and what of this work.]

QA

  • Check out this feature branch
  • Run the site using the command ./run serve or dotrun
  • View the site locally in your web browser at: http://0.0.0.0:8001/
    • Be sure to test on mobile, tablet and desktop screen sizes
  • visit /credentials/schedule?contractItemID=

Issue / Card

Fixes # WD-3904

Screenshots

image

Help

QA steps - Commit guidelines

@webteam-app
Copy link

Demo starting at https://ubuntu-com-12988.demos.haus

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #12988 (451cded) into main (a7d672a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main   #12988   +/-   ##
=======================================
  Coverage   74.90%   74.90%           
=======================================
  Files         107      107           
  Lines        2873     2873           
  Branches      928      928           
=======================================
  Hits         2152     2152           
  Misses        699      699           
  Partials       22       22           

@camillet16
Copy link

@abhigyanghosh30 where can I view the demo, demohaus only leads to ubuntu.com when I add the '/credentials/schedule?contractItemID=' it's a 404.

@abhigyanghosh30
Copy link
Contributor Author

@abhigyanghosh30 where can I view the demo, demohaus only leads to ubuntu.com when I add the '/credentials/schedule?contractItemID=' it's a 404.

@camillet16 Try this link

Copy link

@camillet16 camillet16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhigyanghosh30 here's some reviews for the html i'll leave a CSS comment in a bit.

templates/credentials/schedule.html Outdated Show resolved Hide resolved
templates/credentials/schedule.html Outdated Show resolved Hide resolved
templates/credentials/schedule.html Outdated Show resolved Hide resolved
@camillet16
Copy link

Screen Shot 2023-06-26 at 6 09 37 PM
  • margin-bottom: 1.5rem on top of the 1.2 rem of the <select>

This increases the separation between the selections and the next label. It would total 2.7rem.

@camillet16
Copy link

The background color is #f3f3f3 or $color-paper. thank you!

@abhigyanghosh30
Copy link
Contributor Author

The background color is #f3f3f3 or $color-paper. thank you!

This is for the entire page or the input boxes? Because currently, the input boxes have #f2f2f2 as the default colour.

@abhigyanghosh30
Copy link
Contributor Author

abhigyanghosh30 commented Jun 26, 2023

Does the 2.7rem margin apply to all the input sections?

@camillet16
Copy link

The background color is #f3f3f3 or $color-paper. thank you!

This is for the entire page or the input boxes? Because currently, the input boxes have #f2f2f2 as the default colour.

The input box color with paper background is adjusted to rgba(0,0,0,.04)

@camillet16
Copy link

Does the 2.7rem margin apply to all the input sections?

Yes 😄

@abhigyanghosh30
Copy link
Contributor Author

@camillet16 made changes as requested. Please review again

static/sass/styles.scss Outdated Show resolved Hide resolved
@abhigyanghosh30 abhigyanghosh30 requested review from albertkol and removed request for camillet16 July 26, 2023 12:27
@abhigyanghosh30 abhigyanghosh30 merged commit 43dca33 into main Jul 26, 2023
28 checks passed
@abhigyanghosh30 abhigyanghosh30 deleted the WD-3904-credentials-schedule branch July 26, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants