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

Tooltip not displayed in Workspace name #5907

Closed
isagoico opened this issue Oct 16, 2021 · 21 comments
Closed

Tooltip not displayed in Workspace name #5907

isagoico opened this issue Oct 16, 2021 · 21 comments
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Weekly KSv2

Comments

@isagoico
Copy link

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


Issue is failing PR #5812

Action Performed:

  1. Navigate to workspace settings
  2. Hover over the workspace name

Expected Result:

Tooltip should appear with the full workspace name

Actual Result:

Tooltip does not appear.

Workaround:

None needed, visual issue.

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.1.8-0

Reproducible in staging?: Yes
Reproducible in production?: Yes (but should be fixed in PR #5812)

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

Notes/Photos/Videos: Any additional supporting documentation
image

Expensify/Expensify Issue URL:

Issue reported by: Applause
Slack conversation:

View all open jobs on GitHub

@isagoico isagoico added DeployBlockerCash This issue or pull request should block deployment Engineering Daily KSv2 labels Oct 16, 2021
@MelvinBot
Copy link

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

@OSBotify
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.

@akshayasalvi
Copy link
Contributor

akshayasalvi commented Oct 16, 2021

It is working fine in the main branch.

Screenshot 2021-10-16 at 4 00 23 PM

Screenshot 2021-10-16 at 4 00 11 PM

I can find the Tooltip import but not the Tooltip usage part in the deployed tag 1.1.7-25

Is it not deployed or got removed in some merge conflict?

@tgolen
Copy link
Contributor

tgolen commented Oct 18, 2021

I agree that it looks like the code wasn't deployed properly. I don't think this should be a deploy blocker so I am demoting the issue and my suggestion is to re-evaluate this after the next deploy to staging. If it's still not there, then we'll need to figure out how to fix it properly for staging.

@tgolen tgolen added Weekly KSv2 Improvement Item broken or needs improvement. and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Oct 18, 2021
@Beamanator
Copy link
Contributor

Beamanator commented Oct 18, 2021

From this blame in staging, it looks like the commit adding the Tooltip (A) was made before a more recent merge commit (B), so it got overwritten

Screen Shot 2021-10-18 at 2 32 00 PM

And looking at that same file's blame in main, you can see that the commit "Added tooltip for workspace name" does completely exist there:

Screen Shot 2021-10-18 at 2 34 11 PM

@tgolen
Copy link
Contributor

tgolen commented Oct 18, 2021

Ah, nice investigation, Alex. So, it looks like we just need to get it added back into the code then?

@Beamanator
Copy link
Contributor

Beamanator commented Oct 18, 2021

The part that baffles me is that the commit "Added tooltip for workspace name" actually does seem to exist in staging (see screenshot) but only the tooltip import line!:

Screen Shot 2021-10-18 at 2 36 22 PM

@Beamanator
Copy link
Contributor

Beamanator commented Oct 18, 2021

Clicking "View at this point in history" on the "Added tooltip for workspace name" commit on this page of historical commits, you can see the entire commit exists (importing Tooltip & using the Tooltip).

There's only one more commit on that file in the history (see link above), which is 9295307#diff-f4168366e38bc17178db9db7be5a28974510a61b5b657dcfe9cc86ddc2b51677, a "merge commit" -> So I think that one mucked up somehow - as seen below

Screen Shot 2021-10-18 at 2 45 02 PM

@Beamanator
Copy link
Contributor

cc @roryabraham @AndrewGable - have you gents seen this before?

@roryabraham
Copy link
Contributor

I don't think so ... but great investigation so far! My guess is that someone had to manually resolve conflicts when a PR was CP'd to staging, and accidentally got rid of the tooltip.

@roryabraham
Copy link
Contributor

roryabraham commented Oct 18, 2021

Found it (https://github.com/Expensify/App/pull/5735/files#diff-f4168366e38bc17178db9db7be5a28974510a61b5b657dcfe9cc86ddc2b51677L141-L151) and it was indeed just a bad merge

@Beamanator
Copy link
Contributor

Good find! Let me know how I can help fixing it, if you want help :D

@roryabraham
Copy link
Contributor

Sounds good! I'll submit a PR right now and we'll CP it (this is causing other deploys to fail)

@roryabraham roryabraham assigned roryabraham and unassigned tgolen Oct 18, 2021
@roryabraham
Copy link
Contributor

Wait a minute ... how can I submit a PR if the code's already on main? 🙃 Need a new plan 😆

@Beamanator
Copy link
Contributor

Yeahhh exactly 😅 🙃

@Beamanator
Copy link
Contributor

Beamanator commented Oct 18, 2021

What if we have a commit remove it on main, then another commit adds it back, all in 1 PR?

@roryabraham
Copy link
Contributor

Okay, I think just going to manually CP https://github.com/Expensify/App/pull/5812/files and that should fix it.

@roryabraham
Copy link
Contributor

@roryabraham
Copy link
Contributor

Confirmed this is fixed on staging web, going to close this out.

image

@Beamanator
Copy link
Contributor

Awesome, thanks @roryabraham

@tgolen
Copy link
Contributor

tgolen commented Oct 18, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants