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

[HOLD for payment 2021-09-15] Log in - Language picker has a double lined border #4972

Closed
isagoico opened this issue Sep 1, 2021 · 18 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Sep 1, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Use the language picker

Expected Result:

Language picker has a regular border

Actual Result:

Language picker has a double lined border

Workaround:

N/A. Visual issue.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.0.90-2

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

View all open jobs on GitHub


Found while retesting #4958 (comment) - added the external label because of @roryabraham comment below.

@isagoico isagoico added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 labels Sep 1, 2021
@MelvinBot MelvinBot added Weekly KSv2 and removed Daily KSv2 labels Sep 1, 2021
@MelvinBot
Copy link

Triggered auto assignment to @Luke9389 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@MelvinBot
Copy link

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Sep 1, 2021
@jliexpensify
Copy link
Contributor

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 1, 2021
@meetmangukiya
Copy link
Contributor

I think this happens with all ExpensiPicker that are size small.

Proposal

We have to set border: none for web, and remove border styling from inputWeb, inputIOS, inputAndroid like we are doing for normal sized components here

App/src/styles/styles.js

Lines 591 to 596 in 1f332a6

inputWeb: {
appearance: 'none',
cursor: disabled ? 'not-allowed' : 'pointer',
border: 'none',
...expensiPicker,
},
This also means that whole styles.picker remains unused and should be deleted IMO, was very confusing until I figured it wasn't used.

App/src/styles/styles.js

Lines 314 to 362 in 1f332a6

picker: {
inputIOS: {
flex: 1,
borderWidth: 1,
borderRadius: variables.componentBorderRadiusNormal,
borderColor: themeColors.border,
paddingTop: 25,
paddingHorizontal: 12,
paddingBottom: 8,
justifyContent: 'center',
height: '100%',
backgroundColor: themeColors.componentBG,
},
inputWeb: {
fontFamily: fontFamily.GTA,
fontSize: variables.fontSizeNormal,
paddingLeft: 12,
paddingRight: 12,
paddingTop: 10,
paddingBottom: 10,
borderWidth: 1,
borderRadius: variables.componentBorderRadius,
borderColor: themeColors.border,
color: themeColors.text,
appearance: 'none',
height: variables.inputComponentSizeNormal,
opacity: 1,
cursor: 'pointer',
},
inputAndroid: {
fontFamily: fontFamily.GTA,
fontSize: variables.fontSizeNormal,
paddingLeft: 12,
paddingRight: 12,
paddingTop: 10,
paddingBottom: 10,
borderWidth: 1,
borderRadius: variables.componentBorderRadius,
borderColor: themeColors.border,
color: themeColors.text,
height: variables.inputComponentSizeNormal,
opacity: 1,
},
iconContainer: {
top: 12,
right: 12,
pointerEvents: 'none',
},
},

@roryabraham

This comment has been minimized.

@roryabraham

This comment has been minimized.

@shawnborton
Copy link
Contributor

I posted about this in Slack and it looks like @parasharrajat is already taking care of this: #4810

@parasharrajat
Copy link
Member

Sorry, Shawn but I didn't notice the double border on firefox so this issue was missed. I can send a follow-up, if needed.

@shawnborton
Copy link
Contributor

I think we should follow up and fix it - ideally the border behavior here looks like our text inputs.

@parasharrajat
Copy link
Member

Sure, sending a follow up.

@gnylytsya
Copy link

submit

@parasharrajat
Copy link
Member

PR is here #4986.

@jliexpensify
Copy link
Contributor

jliexpensify commented Sep 2, 2021

Hi @Luke9389 / @roryabraham / @shawnborton - just a little confused as to what's happening!

My understanding is that Rajat has already submitted a PR that will resolve this issue.

Am I paying Rajat as he's fixing it?

@roryabraham
Copy link
Contributor

Okay, I'd actually say that this is a duplicate of #4513, specifically this line-item from the expected result:

The picker theme should match with the theme of other Pickers.

Given that:

We should just close out this issue and the associated Upwork job.

@jliexpensify
Copy link
Contributor

Cheers, job closed.

@botify
Copy link

botify commented Sep 3, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@botify
Copy link

botify commented Sep 4, 2021

🚀 Deployed to staging by @Luke9389 in version: 1.0.93-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Sep 8, 2021
@botify botify changed the title Log in - Language picker has a double lined border [HOLD for payment 2021-09-15] Log in - Language picker has a double lined border Sep 8, 2021
@botify
Copy link

botify commented Sep 8, 2021

🚀 Deployed to production by @yuwenmemon in version: 1.0.94-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants