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.

Sign up for GitHub

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

Jump to bottom

fix: Fixes tests and css #744

Merged
merged 4 commits into from Jul 2, 2019
Merged

fix: Fixes tests and css #744

merged 4 commits into from Jul 2, 2019

Conversation

manueltanzi-okta
Copy link
Contributor

@manueltanzi-okta manueltanzi-okta commented Jul 1, 2019 •
edited

Description

  • Fixes some failing tests after rebase
  • Fixes protractor failing tests
  • Fixes css bug

Screenshots

BEFORE:

Screen Shot 2019-07-01 at 3 19 08 PM

AFTER:

Screen Shot 2019-07-02 at 3 52 21 PM

Screen Shot 2019-07-02 at 3 52 03 PM

Screen Shot 2019-07-02 at 3 50 39 PM

Reviewers

@haishengwu-okta @robertdamphousse-okta @vijetmahabaleshwar-okta

@manueltanzi-okta manueltanzi-okta changed the title fix: Fix tests and css fix: Fixes tests and css Jul 1, 2019
@haoyuli-okta
Copy link
Contributor

@manueltanzi-okta By looking at mocked UI of After, I believe we should not have Enter Code over there. If you look back to the jira again, Enter Code is listed within the textbox

@manueltanzi-okta
Copy link
Contributor Author

cc: @alexdahl-okta or @maggielaw-okta , can you take a look as well? Thank you!

@maggielaw-okta
Copy link

maggielaw-okta commented Jul 2, 2019 •
edited

@manueltanzi-okta thanks for the invitation to review. (@alexdahl-okta is on PTO, FYI.)

It's great to see the alignment issue fixed. How easy would it be to apply an add'l fix to these form elements to improve a11y for focus states? See https://oktainc.atlassian.net/browse/OKTA-210992 and the attached screenshot for similar fixes made on the SIW.
Focus States (OKTA-210992)

@vijetmahabaleshwar-okta
Copy link
Contributor

@manueltanzi-okta By looking at mocked UI of After, I believe we should not have Enter Code over there. If you look back to the jira again, Enter Code is listed within the textbox

@haoyuli-okta - The accessibility changes we've made move the text out of the text box. We will have labels instead which is as shown in the screenshot.

@maggielaw-okta
Copy link

Agree that the positioning of the Enter Code label is confusing. Is there a reason we can't swap the field and button?
image

@manueltanzi-okta
Copy link
Contributor Author

@maggielaw-okta Good point, swapped button and field!
For the focus one, is a bit out of scope for this, and I would like to limit further changes since siw 3.0 has been already thoroughly tested.
I created a JIRA for that: https://oktainc.atlassian.net/browse/OKTA-236470

haishengwu-okta
haishengwu-okta approved these changes Jul 2, 2019
View reviewed changes
Copy link
Contributor

@haishengwu-okta haishengwu-okta left a comment

Choose a reason for hiding this comment

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

👏

@manueltanzi-okta manueltanzi-okta merged commit e01973c into okta:dev-3.0 Jul 2, 2019
manueltanzi-okta added a commit that referenced this pull request Jul 3, 2019
@manueltanzi-okta
fix: Fixes tests and css (#744)
e189ad8 
* fix: Fixes failing tests
* fix: Fixes css
* Fixes e2e tests
manueltanzi-okta added a commit that referenced this pull request Jul 3, 2019
@manueltanzi-okta
fix: Fixes tests and css (#744)
acdde8b 
* fix: Fixes failing tests
* fix: Fixes css
* Fixes e2e tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haishengwu-okta haishengwu-okta haishengwu-okta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@manueltanzi-okta @haoyuli-okta @maggielaw-okta @vijetmahabaleshwar-okta @haishengwu-okta