Skip to content
This repository was archived by the owner on Mar 4, 2025. It is now read-only.

issue 2868 fix #248

Merged

Conversation

PrakashDurlabhji
Copy link

No description provided.

@nkumar-topcoder
Copy link
Contributor

change PR to appirio-tech:qa-accessibility

@mishacucicea mishacucicea self-assigned this Jul 22, 2019
Copy link
Contributor

@mishacucicea mishacucicea left a comment

Choose a reason for hiding this comment

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

@PrakashDurlabhji This applies to the Join button as well. Please make sure the button is highlighted when navigated to (you might need to fix tabindex as well)

@PrakashDurlabhji PrakashDurlabhji changed the base branch from dev to qa-accessibility July 24, 2019 08:27
@PrakashDurlabhji
Copy link
Author

@mishacucicea PR updated

@mishacucicea
Copy link
Contributor

@PrakashDurlabhji Please don't use hardcoded values for colors, they are already present in tc-ui-variables.scss. Reference that file and use variable name.

@PrakashDurlabhji
Copy link
Author

@veshu @mishacucicea PR updated. not able to change the label else it is completed.

@mishacucicea
Copy link
Contributor

@PrakashDurlabhji Your code is not compiling, you need to reference tc-ui-variables.scss

Please make sure you thoroughly check your PRs and changes, otherwise we won't accept your submissions and the level of trust in your code will decrease.

@PrakashDurlabhji
Copy link
Author

@mishacucicea chcking

@PrakashDurlabhji
Copy link
Author

@mishacucicea updated

@mishacucicea
Copy link
Contributor

Undefined variable: "$tc-dark-blue-70". Fix it

@PrakashDurlabhji
Copy link
Author

@mishacucicea isnt a PR merged in it which inserted $tc-dark-blue-70, so i used it assuming it will be present in code till my code is reviewed. I guess it is not yet inside yet.

@mishacucicea
Copy link
Contributor

@PrakashDurlabhji You need to make sure that the current branch, (e.g. issue_2868) is working OK, which, at the time being it doesn't. Currently it shows that these are your changes compared to the qa-accessibility branch. Even if the import was added in another place, if it's not on the qa-accessibility branch you need to fix it.
I'm curious how did you tested locally, you do know changes need to be tested before requesting a review, right?

@PrakashDurlabhji
Copy link
Author

@mishacucicea PR updated with screenshot on issue page and no compilation error now. kindly provide feedback if anything left will confirm everything with screenshot from nowon

@crazyk07 crazyk07 added the P0 label Jul 29, 2019
Copy link
Contributor

@mishacucicea mishacucicea left a comment

Choose a reason for hiding this comment

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

looks good.

@nkumar-topcoder nkumar-topcoder merged commit 1e1c882 into topcoder-archive:qa-accessibility Jul 30, 2019
mishacucicea pushed a commit that referenced this pull request Jul 31, 2019
nkumar-topcoder added a commit that referenced this pull request Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants