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

resolve ux issues #740

Merged
merged 2 commits into from
Feb 17, 2021
Merged

resolve ux issues #740

merged 2 commits into from
Feb 17, 2021

Conversation

amitpatra91
Copy link
Contributor

@amitpatra91 amitpatra91 commented Feb 11, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added
  • Docs have been added or updated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Style Update (CSS)
  • Other... Please describe:

Resolves 722

What is the new behavior?

  1. Alignment issue when options are displaying on top under the modal popup when I have used appendTo: 'body' code.
  2. Clear icon is not clickable when the selected value overlaps with the clear icon.
  3. Also down arrow button overlapping with the selected value.

Does this PR introduce a breaking change?

  • Yes
  • No

Comment on lines 177 to 179
.ng-dropdown-panel.ng-select-bottom {
border: 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure showing the white border is the route we want to take here. It's pretty bold. I looked through the DSM to see if we have dark selects documented anywhere and I couldn't find any. I reached out to @mobi/tangoe-ux-ui to see if they can provide some insight on what we should be doing here.

Copy link
Contributor Author

@amitpatra91 amitpatra91 Feb 12, 2021

Choose a reason for hiding this comment

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

Okay. Please let me know if there are any changes required.

Copy link
Contributor

@grahamhency grahamhency Feb 15, 2021

Choose a reason for hiding this comment

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

@amitpatra91 I merged some SCSS changes to the select the other day and it caused merge conflicts with this branch. I went ahead and rebased with dev and fixed those conflicts for you and force pushed up the new changes.

These 3 lines actually ended up being added back, but we're still waiting to hear back from @mobi/tangoe-ux-ui on what route to take for styles here. When they get back with us we can proceed with this PR.

Alternatively, if you want to merge this as is and create an issue for the dark themed ng-select borders and handle it in a separate PR that would also work and allow us to merge these fixes with the next release. I'll leave it up to you how you want to do it.

If you make additional changes to this PR, make sure you pull down the latest changes first!

Copy link
Contributor

Choose a reason for hiding this comment

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

Per Teams:

Amit:
I will create separate pull request for go-select dark theme related changes. Please release the remaining css changes. Issue #755 created to address this.

@grahamhency grahamhency self-requested a review February 17, 2021 15:02
@grahamhency grahamhency merged commit 1042af7 into dev Feb 17, 2021
@grahamhency grahamhency deleted the go-select_ux_issues branch February 17, 2021 15:02
@grahamhency grahamhency mentioned this pull request Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go-select drop down issues
2 participants