[HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list #33318

lanitochka17 · 2023-12-19T21:49:03Z

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

Version Number: 1.4.14-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: App is setup on Light theme

Navigate to staging.new.expensify.com
Start Request money> Manual flow
On Manual page select Merchant
Use Merchant from suggestion link

Expected Result:

Selected Merchant should be visible in input field

Actual Result:

Selected Merchant is not visible in input field. Letters are light just like the theme

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Bug6320244_1703017103807.Recording__1596.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01704ce5a9ef62e618
Upwork Job ID: 1737228586984062976
Last Price Increase: 2023-12-19
Automatic offers:

aimane-chnaif | Reviewer | 28066426
tienifr | Contributor | 28066427

melvin-bot · 2023-12-19T21:49:07Z

Job added to Upwork: https://www.upwork.com/jobs/~01704ce5a9ef62e618

melvin-bot · 2023-12-19T21:49:07Z

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-12-19T21:49:09Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-12-19T21:49:13Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

ZhenjaHorbach · 2023-12-19T22:56:36Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Theme - Input in Merchant field isn't visible in light mode when select item from suggest list

What is the root cause of that problem?

For text from suggested list we use custom styles
In our case we have a white fill color

App/web/index.html

Lines 93 to 97 in df98114

    
           select:-webkit-autofill:focus { 
        
               -webkit-background-clip: text; 
        
               -webkit-text-fill-color: #ffffff; 
        
               caret-color: #ffffff; 
        
           }

I also think that the problem is also relevant for the caret

What changes do you think we should make in order to solve the problem?

We have several ways

Easy
Just choose a neutral color that will suit the light and dark theme
Easy (2)
We can just add a little shadow
Which will contrast perfectly with the background and will not affect the dark theme in any way
Easy (3)(Looks dangerous)
We can delete this styles
I understand why this was added
Because before we use specific inputs
But now I think This is not very relevant

Harder
During project initialization and during theme selection, we can customize our styles
And depending on the theme, use a specific color

Similar to what we do for a splash screen

App/src/libs/BootSplash/index.ts

Lines 12 to 15 in df98114

    
           const splash = document.getElementById('splash'); 
        
           if (splash) { 
        
               splash.style.opacity = '0'; 
        
           }

But these are all just suggestions
Because we need the designers' opinion

What alternative solutions did you explore? (Optional)

NA

Update

So that there are no disputes in the future that someone spied on someone
One of my options is similar to the second proposition
Which were sent at the same time ))

Screen.Recording.2023-12-20.at.00.18.53.mov

yh-0218 · 2023-12-19T23:04:34Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Theme - Input in Merchant field isn't visible in light mode when select item from suggest list

What is the root cause of that problem?

We have custom autofill color.

App/web/index.html

Line 95 in aa1d87c

-webkit-text-fill-color: #ffffff;

What changes do you think we should make in order to solve the problem?

We need to variable for this.

     -webkit-background-clip: text;
     -webkit-text-fill-color: var(--text-color);
     caret-color: var(--text-color);

and need to set --text-color on style of BaseTextInput here

{'--text-color': theme.text}

Screen.Recording.2023-12-20.at.1.28.26.PM.mov

What alternative solutions did you explore? (Optional)

tienifr · 2023-12-20T05:44:50Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Selected Merchant is not visible in input field. Letters are light just like the theme.

What is the root cause of that problem?

In here, we're setting the white color to autofilled inputs, this is to fix a bug where sometimes browsers add special styles to text inputs if those inputs are autofilled, for example Chrome on iOS will add yellow color.

This will work fine for the default dark theme we have earlier, but since now light theme is supported, the white color on light theme will cause UI problem.

In fact we should not set it as #ffffff, we should set it equal to the text color of that theme, in case of dark mode it will be #002E22 (white-ish but not white).

Also the reason why we have to put it in CSS is because there's no native way in React to target the autofill pseudo class of an input.

What changes do you think we should make in order to solve the problem?

We need to use different CSS with correct theme colors.

To do that we can (the below changes are only needed for web platform):

Add a new addCSS method to addCSS during run time, it will look like below (the script might seem long but in simple terms, it will replace existing style tag if it already exists, if not it will append another style tag with the correct CSS), we can try to simply it a bit later during PR process.

function addCSS(css, styleId) {
    var head = document.getElementsByTagName('head')[0];
    var existingStyle = document.getElementById(styleId);

    if (existingStyle) {
        // If style tag with the specified ID exists, update its content
        if (existingStyle.styleSheet) {   // IE
            existingStyle.styleSheet.cssText = css;
        } else {                          // the world
            existingStyle.innerHTML = css;
        }
    } else {
        // If style tag doesn't exist, create a new one
        var s = document.createElement('style');
        s.setAttribute("id", styleId);
        s.setAttribute('type', 'text/css');

        if (s.styleSheet) {   // IE
            s.styleSheet.cssText = css;
        } else {              // the world
            s.appendChild(document.createTextNode(css));
        }

        head.appendChild(s);
    }
}

In here, add an useEffect to listen to theme change (or more specifically theme.text change). If it changes, add the proper autofill CSS with the correct theme.text color

useEffect(() => {
        addCSS(getAutofilledInputStyle(theme.text), 'autofill-input')
}, [theme.text]);

We can add getAutofilledInputStyle as a style util for use above

const getAutofilledInputStyle = (inputTextColor) => {
    return `
        input[chrome-autofilled],
        input[chrome-autofilled]:hover,
        input[chrome-autofilled]:focus,
        textarea[chrome-autofilled],
        textarea[chrome-autofilled]:hover,
        textarea[chrome-autofilled]:focus,
        select[chrome-autofilled],
        select[chrome-autofilled]:hover,
        select[chrome-autofilled]:focus,
        input:-webkit-autofill,
        input:-webkit-autofill:hover,
        input:-webkit-autofill:focus,
        textarea:-webkit-autofill,
        textarea:-webkit-autofill:hover,
        textarea:-webkit-autofill:focus,
        select:-webkit-autofill,
        select:-webkit-autofill:hover,
        select:-webkit-autofill:focus {
            -webkit-background-clip: text;
            -webkit-text-fill-color: ${inputTextColor};
            caret-color: ${inputTextColor};
        }
    `
}

The above are reusable for any case where we need to use theme-specific CSS.

What alternative solutions did you explore? (Optional)

NA

aimane-chnaif · 2023-12-20T06:59:19Z

@tienifr thanks for the proposal. Can you share your branch for testing?

tienifr · 2023-12-20T07:29:25Z

@tienifr thanks for the proposal. Can you share your branch for testing?

@aimane-chnaif sure thing! here you go https://github.com/Expensify/App/compare/main...tienifr:fix/33318?expand=1

It's working fine below

Screen.Recording.2023-12-20.at.2.31.22.PM.mov

bernhardoj · 2023-12-20T07:37:43Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The merchant input text is not visible if we auto-fill it (light mode)

What is the root cause of that problem?

We hard-coded a style for the auto-fill input to have a white color.

App/web/index.html

Lines 76 to 97 in 52ecad4

    
           input[chrome-autofilled], 
        
           input[chrome-autofilled]:hover, 
        
           input[chrome-autofilled]:focus, 
        
           textarea[chrome-autofilled], 
        
           textarea[chrome-autofilled]:hover, 
        
           textarea[chrome-autofilled]:focus, 
        
           select[chrome-autofilled], 
        
           select[chrome-autofilled]:hover, 
        
           select[chrome-autofilled]:focus, 
        
           input:-webkit-autofill, 
        
           input:-webkit-autofill:hover, 
        
           input:-webkit-autofill:focus, 
        
           textarea:-webkit-autofill, 
        
           textarea:-webkit-autofill:hover, 
        
           textarea:-webkit-autofill:focus, 
        
           select:-webkit-autofill, 
        
           select:-webkit-autofill:hover, 
        
           select:-webkit-autofill:focus { 
        
               -webkit-background-clip: text; 
        
               -webkit-text-fill-color: #ffffff; 
        
               caret-color: #ffffff; 
        
           }

So, when we auto-fill it in light mode, we won't be able to see it.

To solve this issue, we need to understand first the history of the above style.

On most browsers, they have a default auto-fill style that is not overridable.

On the Chromium browser, the color is set to fieldtext system color. fieldtext value changes based on color-scheme value. If it's dark, the fieldtext changes to white, if it's light, the fieldtext changes to black.

The default color-scheme is normal which has the same fieldtext value as light, which is black.

However, at the time we applied a dark theme to the app for the first time, we didn't have a color-scheme style set yet. So, the auto-fill text color is still black which then raises this issue because at that time, the input background color is black and so this PR (and this) update the auto-fill color to white using -webkit-text-fill-color (not color because it's not overridable).

But we now have the color-scheme set, so the fieldtext color will dynamically change based on the color-scheme.

What changes do you think we should make in order to solve the problem?

The simplest solution is to remove the hard-coded auto-fill style, but it will have a 100% white and black color which is not the same as theme.text color (and maybe a different browser could have a different non-overridable color style).

So, what we can do instead is to move the style from index.html to BaseTextInput and BasePicker inputWeb style.

{WebkitBackgroundClip: 'text', caretColor: theme.text},
hasValue ? {WebkitTextFillColor: theme.text} : {},

BaseTextInput will target both input and textarea, while BasePicker will target select.

WebkitTextFillColor will also affect placeholder text color, so we need to set it only if it has a value. (for BaseTextInput only)

With this change, we will set all those 3 styles whether the input is auto-filled or manually filled.

We already have 2 uses of those styles here:

App/src/styles/index.ts

Lines 799 to 806 in 52ecad4

    
           // Adding browser specefic style to bring consistency between Safari and other platforms. 
        
           // Applying the Webkit styles only to browsers as it is not available in native. 
        
           ...(Browser.getBrowser() 
        
               ? { 
        
                     WebkitTextFillColor: theme.textSupporting, 
        
                     WebkitOpacity: 1, 
        
                 } 
        
               : {}),

App/src/styles/index.ts

Lines 2647 to 2657 in 52ecad4

    
           // These properties are available in browser only 
        
           ...(Browser.getBrowser() 
        
               ? { 
        
                     caretColor: 'transparent', 
        
                     WebkitTextFillColor: 'transparent', 
        
                     // After setting the input text color to transparent, it acquires the background-color. 
        
                     // However, it is not possible to override the background-color directly as explained in this resource: https://developer.mozilla.org/en-US/docs/Web/CSS/:autofill 
        
                     // Therefore, the transition effect needs to be delayed. 
        
                     transitionDelay: '99999s', 
        
                     transitionProperty: 'background-color', 
        
                 }

aimane-chnaif · 2023-12-20T07:37:44Z

Thanks. It works well

aimane-chnaif · 2023-12-20T07:37:58Z

@tienifr's proposal looks good to me
🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-12-20T07:38:05Z

Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

bernhardoj · 2023-12-20T07:40:20Z

@aimane-chnaif can you review mine, please?

aimane-chnaif · 2023-12-20T07:44:24Z

@bernhardoj just reviewed. I am fine with global approach and if more web css styles (not only input) are needed in the future, we can just use same util.
If you find any regression from @tienifr's solution (i.e. WebkitTextFillColor will also affect placeholder text color, so we need to set it only if it has a value.), I will consider yours.

bernhardoj · 2023-12-20T08:01:10Z

if more web css styles (not only input) are needed in the future, we can just use same util.

I think we won't be allowed anymore to add more theme styles directly to the index.html (except it's unavoidable) and I'm just thinking that it's better if we could have the style set on the react js code, just like we did with color-scheme (by having a ColorSchemeWrapper component).

Also, we want to have the same text color for both auto and manual fill, so I think the auto-fill selector is actually not needed.

(that's just my opinion)

aimane-chnaif · 2023-12-20T08:07:53Z

I think we won't be allowed anymore to add more theme styles directly to the index.html (except it's unavoidable)

yes, I meant unavoidable styles. i.e. #13328

yh-0218 · 2023-12-20T11:30:37Z

Hi @aimane-chnaif Could you check my updated Proposal
I think this is good way. Your opinion?

tienifr · 2023-12-20T11:44:21Z

yes, I meant unavoidable styles. i.e. #13328

Agreed with @aimane-chnaif, basically all pseudo-element style targeting based on theme will have to use a variation of my solution, better add it now 😄

thienlnam · 2023-12-20T21:11:11Z

Assigning you since this is a theme related bug and you have more context on it

melvin-bot · 2023-12-20T21:27:45Z

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

melvin-bot · 2023-12-20T21:27:48Z

📣 @tienifr 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

grgia · 2023-12-20T21:27:55Z

Thank you for the proposal @tienifr, all yours!

tienifr · 2023-12-25T06:59:22Z

PR ready for review #33484.

melvin-bot · 2024-01-17T17:19:43Z

This issue has not been updated in over 15 days. @grgia, @bfitzexpensify, @aimane-chnaif, @tienifr eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

melvin-bot · 2024-01-31T22:40:12Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-01-31T22:40:15Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Theme switch for autofill pseudo class #33484

If no regressions arise, payment will be issued on 2024-02-07. 🎊

For reference, here are some details about the assignees on this issue:

@aimane-chnaif requires payment automatic offer (Reviewer)
@tienifr requires payment automatic offer (Contributor)

melvin-bot · 2024-01-31T22:40:16Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
[@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

aimane-chnaif · 2024-02-04T15:14:04Z

This minor bug came from theme-switching live.
As caught by QA team, we can skip regression test.

bfitzexpensify · 2024-02-07T06:33:19Z

Agreed @aimane-chnaif. Contracts paid out, we're all done here - thanks everyone!

lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 19, 2023

melvin-bot bot assigned bfitzexpensify Dec 19, 2023

melvin-bot bot changed the title ~~Theme - Input in Merchant field isn't visible in light mode when select item from suggest list~~ [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list Dec 19, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 19, 2023

melvin-bot bot assigned aimane-chnaif Dec 19, 2023

melvin-bot bot assigned thienlnam Dec 20, 2023

thienlnam assigned grgia and unassigned thienlnam Dec 20, 2023

grgia assigned tienifr Dec 20, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 20, 2023

tienifr mentioned this issue Dec 22, 2023

Theme switch for autofill pseudo class #33484

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 25, 2023

melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Jan 17, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Monthly KSv2 labels Jan 31, 2024

melvin-bot bot changed the title ~~[$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list~~ [HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list Jan 31, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jan 31, 2024

bfitzexpensify closed this as completed Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list #33318

[HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list #33318

lanitochka17 commented Dec 19, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 19, 2023

melvin-bot bot commented Dec 19, 2023

melvin-bot bot commented Dec 19, 2023

melvin-bot bot commented Dec 19, 2023

ZhenjaHorbach commented Dec 19, 2023 •

edited

Loading

yh-0218 commented Dec 19, 2023 •

edited

Loading

tienifr commented Dec 20, 2023 •

edited

Loading

aimane-chnaif commented Dec 20, 2023

tienifr commented Dec 20, 2023 •

edited

Loading

bernhardoj commented Dec 20, 2023

aimane-chnaif commented Dec 20, 2023

aimane-chnaif commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

bernhardoj commented Dec 20, 2023 •

edited

Loading

aimane-chnaif commented Dec 20, 2023

bernhardoj commented Dec 20, 2023

aimane-chnaif commented Dec 20, 2023

yh-0218 commented Dec 20, 2023

tienifr commented Dec 20, 2023

thienlnam commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

grgia commented Dec 20, 2023

tienifr commented Dec 25, 2023

melvin-bot bot commented Jan 17, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

aimane-chnaif commented Feb 4, 2024

bfitzexpensify commented Feb 7, 2024

[HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list #33318

[HOLD for payment 2024-02-07] [$500] Theme - Input in Merchant field isn't visible in light mode when select item from suggest list #33318

Comments

lanitochka17 commented Dec 19, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 19, 2023

melvin-bot bot commented Dec 19, 2023

melvin-bot bot commented Dec 19, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 19, 2023

ZhenjaHorbach commented Dec 19, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

yh-0218 commented Dec 19, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

tienifr commented Dec 20, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

aimane-chnaif commented Dec 20, 2023

tienifr commented Dec 20, 2023 • edited Loading

bernhardoj commented Dec 20, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

aimane-chnaif commented Dec 20, 2023

aimane-chnaif commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

bernhardoj commented Dec 20, 2023 • edited Loading

aimane-chnaif commented Dec 20, 2023

bernhardoj commented Dec 20, 2023

aimane-chnaif commented Dec 20, 2023

yh-0218 commented Dec 20, 2023

tienifr commented Dec 20, 2023

thienlnam commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

melvin-bot bot commented Dec 20, 2023

grgia commented Dec 20, 2023

tienifr commented Dec 25, 2023

melvin-bot bot commented Jan 17, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

aimane-chnaif commented Feb 4, 2024

bfitzexpensify commented Feb 7, 2024

lanitochka17 commented Dec 19, 2023 •

edited by melvin-bot bot

Loading

ZhenjaHorbach commented Dec 19, 2023 •

edited

Loading

yh-0218 commented Dec 19, 2023 •

edited

Loading

tienifr commented Dec 20, 2023 •

edited

Loading

tienifr commented Dec 20, 2023 •

edited

Loading

bernhardoj commented Dec 20, 2023 •

edited

Loading