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

fix(Xero): Fix showing Toggl Button in Xero #2022

Merged
merged 2 commits into from
Jan 24, 2022
Merged

Conversation

EternallLight
Copy link
Contributor

@EternallLight EternallLight commented Nov 24, 2021

🌟 What does this PR do?

Fixes the broken selectors for Xero, so that Toggl Button appears in place.

🐛 Recommendations for testing

  1. Sign in to Xero (we have test credentials)
  2. Make sure the Toggl Button is visible:

📝 Links to relevant issues or information

#1910

@EternallLight EternallLight self-assigned this Nov 24, 2021
@EternallLight EternallLight linked an issue Nov 24, 2021 that may be closed by this pull request
@EternallLight EternallLight requested a review from a team November 24, 2021 13:31
Copy link
Contributor

@tiberiusuciu tiberiusuciu left a comment

Choose a reason for hiding this comment

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

I kind of already asked for this on the question I asked on your other PR
But for Xero instead 😄

Copy link
Contributor

@tiberiusuciu tiberiusuciu left a comment

Choose a reason for hiding this comment

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

Got the credentials 🙌

I was able to login, but I can't get past the screen that forces me to setup a multi-factor authentication.
image

II don't think I should be the one doing this 😅 Did you manage to get past it? Or did you create one temporarily and then disabled it?

@EternallLight
Copy link
Contributor Author

It didn't ask me to. It might have became suspicious of why two people from different part of the globe logged in on the same day after weeks of no activity :)

@EternallLight
Copy link
Contributor Author

@tiberiusuciu I have double-checked the fix – it wasn't there due to host selector changes and now it is. Looking 👌 as you can see from the screenshot.

Copy link
Contributor

@tiberiusuciu tiberiusuciu left a comment

Choose a reason for hiding this comment

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

I was able to log in with Xero, it seems like this branch does not show the Toggl button on the header:
image

Is it supposed to only be visible on specific pages? 🤔

@EternallLight
Copy link
Contributor Author

@tiberiusuciu it is supposed to be visible where the header is. I've recorded a short demo video: https://cln.sh/SHqBXr

@EternallLight
Copy link
Contributor Author

Rebased and removed double-declarations from origins.js

Copy link
Contributor

@tiberiusuciu tiberiusuciu left a comment

Choose a reason for hiding this comment

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

I think we made this repo private, but I can't seem to find the instruction on how to set up the extension locally to test it out. Could anyone refer me to this documentation? 👀

@EternallLight
Copy link
Contributor Author

EternallLight commented Jan 5, 2022

@tiberiusuciu

I've brought this up in Slack, but here's how I've been able to run it:

  1. Clone git@github.com:toggl/track-extension-core.git and git@github.com:toggl/toggl-button.git side by side.
  2. Do yarn install in the former.
  3. Go to the toggl-button directory and do yarn link
  4. Go to the toggl-extension-core directory and do yarn link "@toggl/track-extension"
  5. Edit the resolve section of webpack.config.js to be the following:
  resolve: {
    extensions: ['.tsx', '.ts', '.js', '.jsx'],
    alias: {
      'webextension-polyfill': path.resolve(__dirname, 'node_modules/webextension-polyfill')
    }
  },

Not sure why, but it doesn't want to include webextension-polyfill from toggl-button/src/content without pointing directly to its location.
6. Run yarn start and enjoy the extension testing as before. When you change files to toggl-button, webpack rebuilds happen automagically.

Copy link
Contributor

@tiberiusuciu tiberiusuciu left a comment

Choose a reason for hiding this comment

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

With the new repos on my machine and with the proper configuration, I was able to see the Toggl button visible on Xero.

It's working now! Let's just address the merge-conflict and this PR is good to go!

@EternallLight EternallLight merged commit 2271678 into master Jan 24, 2022
@EternallLight EternallLight deleted the 1910/xero-fix branch January 24, 2022 12:27
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.

Toggl Button doesn't show up on Xero
2 participants