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

Android & iOS - Categories - Import spreadsheet icon is missing #49885

Closed
2 of 6 tasks
lanitochka17 opened this issue Sep 28, 2024 · 11 comments
Closed
2 of 6 tasks

Android & iOS - Categories - Import spreadsheet icon is missing #49885

lanitochka17 opened this issue Sep 28, 2024 · 11 comments
Assignees
@luacmartins
Labels
Engineering Hourly KSv2 Reviewing Has a PR in review

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 28, 2024 •
edited
Loading

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 9.0.41-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch New Expensify app
  2. Go to workspace settings > Categories
  3. Tap 3-dot menu

Expected Result:

There will be icon for Import spreadsheet

Actual Result:

Import spreadsheet icon is missing

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6617870_1727470860273.ScreenRecording_09-28-2024_04-59-27_1.mp4

View all open jobs on GitHub
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Sep 28, 2024
@melvin-bot Melvin Bot
Copy link

melvin-bot bot commented Sep 28, 2024

Triggered auto assignment to @pecanoro (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions GitHub Actions
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

We think that this bug might be related to #wave-control

@github-actions github-actions bot mentioned this issue Sep 28, 2024
Closed
@Krishna2323
Copy link
Contributor

Krishna2323 commented Sep 28, 2024 •
edited
Loading

Proposal


Offending PR: #49569

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

Android & iOS - Categories - Import spreadsheet icon is missing

What is the root cause of that problem?

fill="none" is applied to the svg element.

App/assets/images/table.svg

Line 1 in 71db3f1

<svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg">

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


We should remove the fill="none" property.

What alternative solutions did you explore? (Optional)

Result

@huult
Copy link
Contributor

huult commented Sep 28, 2024 •
edited by github-actions bot
Loading

Edited by proposal-police: This proposal was edited at 2023-10-05T22:17:00Z.

Proposal

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

Android & iOS - Categories - Import spreadsheet icon is missing

What is the root cause of that problem?

The file SVG included fill="inherit". The issue with fill="inherit" not showing up in React Native on iOS and Android is likely due to how React Native handles SVG properties. Unlike web browsers, React Native doesn’t automatically inherit styles from parent components in the same way.

App/assets/images/table.svg

Line 2 in 71db3f1

<path fill-rule="evenodd" clip-rule="evenodd" d="M0 4C0 2.89543 0.895431 2 2 2H18C19.1046 2 20 2.89543 20 4V16C20 17.1046 19.1046 18 18 18H2C0.895431 18 0 17.1046 0 16V4ZM2 6V10H9V6H2ZM11 6V10H18V6H11ZM9 12H2V16H9V12ZM11 16V12H18V16H11Z" fill="inherit"/>

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

To resolve this, we can use fill="currentColor" (ex: fill="#8B9C8F") instead of fill="inherit". Example:

// ./assets/images/table.svg#L2
<svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg">
- <path fill-rule="evenodd" clip-rule="evenodd" d="M0 4C0 2.89543 0.895431 2 2 2H18C19.1046 2 20 2.89543 20 4V16C20 17.1046 19.1046 18 18 18H2C0.895431 18 0 17.1046 0 16V4ZM2 6V10H9V6H2ZM11 6V10H18V6H11ZM9 12H2V16H9V12ZM11 16V12H18V16H11Z" fill="inherit"/>
+ <path fill-rule="evenodd" clip-rule="evenodd" d="M0 4C0 2.89543 0.895431 2 2 2H18C19.1046 2 20 2.89543 20 4V16C20 17.1046 19.1046 18 18 18H2C0.895431 18 0 17.1046 0 16V4ZM2 6V10H9V6H2ZM11 6V10H18V6H11ZM9 12H2V16H9V12ZM11 16V12H18V16H11Z" fill="#8B9C8F"/>
</svg>
POC
Screen.Recording.2024-09-28.at.22.53.02.mov

@Nodebrute
Copy link
Contributor

Proposal

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

Import spreadsheet icon is missing

What is the root cause of that problem?

We are using fill="inherit" here. We are not using it anywhere else in the app

App/assets/images/table.svg

Line 2 in 71db3f1

<path fill-rule="evenodd" clip-rule="evenodd" d="M0 4C0 2.89543 0.895431 2 2 2H18C19.1046 2 20 2.89543 20 4V16C20 17.1046 19.1046 18 18 18H2C0.895431 18 0 17.1046 0 16V4ZM2 6V10H9V6H2ZM11 6V10H18V6H11ZM9 12H2V16H9V12ZM11 16V12H18V16H11Z" fill="inherit"/>

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

We can remove fill="inherit" and we should also remove fill=none from here

App/assets/images/table.svg

Line 2 in 71db3f1

<path fill-rule="evenodd" clip-rule="evenodd" d="M0 4C0 2.89543 0.895431 2 2 2H18C19.1046 2 20 2.89543 20 4V16C20 17.1046 19.1046 18 18 18H2C0.895431 18 0 17.1046 0 16V4ZM2 6V10H9V6H2ZM11 6V10H18V6H11ZM9 12H2V16H9V12ZM11 16V12H18V16H11Z" fill="inherit"/>

What alternative solutions did you explore? (Optional)

@roryabraham roryabraham removed the DeployBlocker Indicates it should block deploying the API label Sep 29, 2024
@roryabraham roryabraham mentioned this issue Sep 29, 2024
Merged
50 tasks
@ishpaul777

This comment was marked as outdated.

Sign in to view
@ishpaul777 ishpaul777 mentioned this issue Sep 29, 2024
Merged
50 tasks
@ishpaul777
Copy link
Contributor

PR is here #49897

@luacmartins luacmartins assigned luacmartins and unassigned pecanoro Sep 30, 2024
@luacmartins
Copy link
Contributor

I can take this since I reviewed the offending PR.

@luacmartins luacmartins added the Reviewing Has a PR in review label Sep 30, 2024
@jasperhuangg
Copy link
Contributor

Is it safe to retest this on staging and close is out? @luacmartins

@luacmartins
Copy link
Contributor

Confirmed fix on staging.

@luacmartins luacmartins removed the DeployBlockerCash This issue or pull request should block deployment label Sep 30, 2024
@ishpaul777 ishpaul777 mentioned this issue Oct 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@luacmartins luacmartins

Labels
Engineering Hourly KSv2 Reviewing Has a PR in review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@pecanoro @huult @luacmartins @jasperhuangg @roryabraham @lanitochka17 @Krishna2323 @Nodebrute @ishpaul777