topcoder-platform / community-app

React webapp for serving Topcoder Community
125 stars 212 forks source link

[$20][MSFT-59] [Registration] - The color contrast is very low for Join Button. #2775

Closed veshu closed 5 years ago

veshu commented 5 years ago

Steps to Reproduce

  1. Go to https://accounts.topcoder.com/member/registration
  2. Inspect Element
  3. Observe the Join Button color.

Expected Result Expected contrast ratio of 4.5:1.

Actual Result Element has insufficient color contrast of 1.5.1 (foreground color: #FFFFFF, background color: ##d1d3d4)

Device: Desktop/Labtop Operating System: macOS Browser: Chrome

WCAG Category: 1. Perceivable WCAG Level: AA WCAG Success Criterion: 1.4.3 Contrast (minimum) Screen Reader Used: Accessibility Audit Tool used: Other

Attachment https://topcodermsft-my.sharepoint.com/personal/pd-topcoder_topcodermsft_onmicrosoft_com/Documents/Forms/All.aspx?cid=ae14bf9a%2D590b%2D4bb4%2Da40a%2D4c6822c880a4&FolderCTID=0x0120005C598A51049FC14CBA882E1AEE168F51&id=%2Fpersonal%2Fpd%2Dtopcoder%5Ftopcodermsft%5Fonmicrosoft%5Fcom%2FDocuments%2FMSFT%2DTeams%2DQA%2FJune%202019%2F29%20Jun%2FTest%20Results%2FBug%20Videos%20%26%20Log%20Files%2FCreeya%2FBug%2059

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

Contest https://www.topcoder.com/challenges/30096331 has been created for this ticket.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

Contest https://www.topcoder.com/challenges/30096331 has been updated - the new changes has been updated for this ticket.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

Contest https://www.topcoder.com/challenges/30096331 has been updated - it has been assigned to PkDurlabhji.

This is an automated message for lazybaer via Topcoder X

PrakashDurlabhji commented 5 years ago

@codeMinter PR https://github.com/appirio-tech/accounts-app/pull/242

Dara-K commented 5 years ago

That button is disabled now and will become enabled once the user fills all the fields.

tosha5252 commented 5 years ago

(disabled) color changed to #C3C1C1 Contrast Ratio: 1.79:1...does not meet WCAG requirements

PrakashDurlabhji commented 5 years ago

@veshu @tosha5252 why is PR reverted for it? https://github.com/appirio-tech/accounts-app/pull/259 confused

veshu commented 5 years ago

@mishacucicea FYI

mishacucicea commented 5 years ago

@PrakashDurlabhji Please follow the issues/conversations. If an issue it's QA Fail or tcx_Feedback it needs to be addressed. By default a 'QA Fail' needs to be reverted, but if you answer fast it won't be. Please do the adjustments and create another pull request, it's a simple change.

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

[400]: Failed to create challenge. Detail: Operation failed in the contest service facade.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

Contest https://www.topcoder.com/challenges/30097377 has been created for this ticket.

This is an automated message for lazybaer via Topcoder X

cwdcwd commented 5 years ago

Contest https://www.topcoder.com/challenges/30097377 has been updated - it has been assigned to PkDurlabhji.

This is an automated message for lazybaer via Topcoder X

PrakashDurlabhji commented 5 years ago

@mishacucicea PR raised again https://github.com/appirio-tech/accounts-app/pull/267

mishacucicea commented 5 years ago

@PrakashDurlabhji You introduced a color that's not needed and the contrast was still not fixed, so I just changed it myself, seems like it takes too long to explain it, and you still don't use the tools to verify your work. @Dara-K While fixing the colors, I've noticed that disabled buttons have an opacity which will make them less visible, even if the Tool says that contrast is OK. Not sure exactly if we should used other colors. image

mishacucicea commented 5 years ago

@PrakashDurlabhji Please do not pick-up new items to work on if you have pending items already, if needed I will enforce that.

tosha5252 commented 5 years ago

The color has been changed to #5d5d66, but would also like to know from @Dara-K if opacity level needs to be adjusted or removed...@mishacucicea, would another issue be created to change the opacity level, or just use this one?

mishacucicea commented 5 years ago

@tosha5252 - We'll use a new one if needed, to keep it simple.

crazyk07 commented 5 years ago

Contest https://www.topcoder.com/challenges/30097377 has been updated - it has been assigned to PkDurlabhji.

This is an automated message for crazyk via Topcoder X

Dara-K commented 5 years ago

For this one, let's change the color of the Join button and Show button when disabled to #6B6B6B. No need to use transparency for them as that will lower the contrast only and will make it difficult to perceive for the users.

When active, they should both be blue color #006AD7

PrakashDurlabhji commented 5 years ago

@mishacucicea sorry for delay but all will be tested now and perfect, wont take time now. I am raising PR now which satisfies above comment of Dara-K, I also saw your commits on my PR which is merged now, thanks for that.

PrakashDurlabhji commented 5 years ago

@mishacucicea @tosha5252 new PR and locally tested perfectly. https://github.com/appirio-tech/accounts-app/pull/278

mishacucicea commented 5 years ago

@PrakashDurlabhji

I have fix it, take a look at the PR, hope you learn from it. Waiting for @nkumar-topcoder to merge the code.

PrakashDurlabhji commented 5 years ago

@mishacucicea thanks for help but you said we will handle opacity in another ticket so I did not touch it.

drasticdpk commented 5 years ago

The color code suggested by @Dara-K has not been updated. image

PrakashDurlabhji commented 5 years ago

@drasticdpk PR is perfect and values are updated as darak commented

drasticdpk commented 5 years ago

Hi @PrakashDurlabhji , Below is my finding , pease suggest if i was wrong. It would be great if you share your finding screenshot too. https://monosnap.com/file/xn2B7DuEzjALghY11ju9KgsS2CtEHZ https://monosnap.com/file/fzqalFkSaMBriTYYdFqW1lfNxubcbq

drasticdpk commented 5 years ago

@PrakashDurlabhji @Oanh-and-only-Oanh it was pass on test server. Verified on Google chrome.

nithyaasworld commented 5 years ago

Its failing in the following places:

  1. Show button in disabled state
  2. Join button in the following scenario: a: Just enter "a" in the country field and go to any other field b: It shows a message asking the user to choose country from the list c: Now enter another "a" and choose a country from the list d: Check the Join button disabled state not being in WCAG Standards

image

image

Expectation is:

For this one, let's change the color of the Join button and Show button when disabled to #6B6B6B. No need to use transparency for them as that will lower the contrast only and will make it difficult to perceive for the users.

When active, they should both be blue color #006AD7

nithyaasworld commented 5 years ago

@PrakashDurlabhji Please check above. Thanks.

nithyaasworld commented 5 years ago

@PrakashDurlabhji, Please wait.

CC: @nkumar-topcoder

nkumar-topcoder commented 5 years ago

@PrakashDurlabhji can you pls look into above comment, fix and test locally. Note:

  1. tc-white-cream is not there or defined, temp. i added tc-whitepls ensure that you add relevant dependency

  2. Very important - Ensure that you are talking latest files ONLY for this issue fix from DEV Branch. Else, merging will be a headache. @mishacucicea can you verify

nkumar-topcoder commented 5 years ago

@PrakashDurlabhji @mishacucicea #2868, #2775 and #2774 all fix overlap with similar files, earlier merges failed. Can you please look into The PR are 248, 278.

PrakashDurlabhji commented 5 years ago

@nithyaasworld

  1. I went through your comment https://github.com/topcoder-platform/community-app/issues/2775#issuecomment-516722407

and for the same additional comment has been added at https://github.com/appirio-tech/accounts-app/pull/278/commits/1875bfd0134534eff7da91eb5fda593483f2f4f8

@mishacucicea and me will sync it to make it work and satisfy requirements.

  1. Regarding $tc-white-cream, i can see that variable is present in "qa-accessibility" branch but not in "dev" branch. hence it is saying $tc-white-cream not present

and I guess it is removed by someone while resolving and merging conflicts. lets keep that aside and lets complete this immediately.

I may reply a bit late but kindly do not assign all my 3 issues to someone else, I will complete ASAP.

Note: I am just confused regardign taking latest from dev? means I need to make a new PR which I am aware:

  1. so I need to take latest from dev branch?
  2. and make code changes, but raise PR against "qa-accessiblity"?
  3. or I need to make changes, and raise PR against "dev" branch itself?

kindly confirm on above.

@veshu

nkumar-topcoder commented 5 years ago

@PrakashDurlabhji there could be other fixes which might have been already pushed to DEV branch for the file you are about to fix. This causes merge issue later and latest fix will overwrite old fix causing another issue. If there is no conflict on that file take form qa-accessibilty only. You need to take care of this. Hope this clarifies.

Do not raise any PR against DEV. Fix all 3 issues, test locally and raise PR for qa-accessibility.

FYI @mishacucicea

PrakashDurlabhji commented 5 years ago

@nkumar-topcoder yes got some idea, trying it. Though it is difficult.

crazyk07 commented 5 years ago

Contest https://www.topcoder.com/challenges/30097377 has been updated - the new changes has been updated for this ticket.

This is an automated message for crazyk via Topcoder X

lakshmiathreya commented 5 years ago
Screenshot 2019-08-05 at 11 24 21 AM Screenshot 2019-08-05 at 11 24 06 AM

Please see the screenshots attached - The disabled Join button has different colors when Country is selected and it is yet to be selected. Disabled Join button should have consistent color right - pls clarify @Dara-K. fyi @nithyaasworld @tosha5252 @Dara-K @nkumar-topcoder

PrakashDurlabhji commented 5 years ago

@veshu apology for delay,I am making PR for this and other issues connected with this.

lakshmiathreya commented 5 years ago

Partially filled password also has the same issue. Pls see the screenshot.

Screenshot 2019-08-05 at 11 51 53 AM
tosha5252 commented 5 years ago

country

Witnessing the same as @lakshmiathreya...when the country is picked, button changes

PrakashDurlabhji commented 5 years ago

@tosha5252 @lakshmiathreya yes because buttons are different in both cases and code was removed hence changes not visible.

tosha5252 commented 5 years ago

country2 check the code....twice for the same button...the color is not right in one case

crazyk07 commented 5 years ago

Contest https://www.topcoder.com/challenges/30097377 has been updated - it has been assigned to rohitgupta_.

This is an automated message for crazyk via Topcoder X

mishacucicea commented 5 years ago

@PrakashDurlabhji , I've assigned to @r0hit-gupta as we need a fast fix. Please contact me on Slack to discuss about compensating for part of your work on this item. If you are not on Topcoder Community Slack, please register using this link.

r0hit-gupta commented 5 years ago

@mishacucicea @nkumar-topcoder PR at https://github.com/appirio-tech/accounts-app/pull/294 I have also fixed the Terms & Privacy Policy not highlighting on tab focus. Please review.

image