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

Add unpublished Safari build #3677

Merged
merged 26 commits into from
Oct 25, 2020
Merged

Add unpublished Safari build #3677

merged 26 commits into from
Oct 25, 2020

Conversation

fregante
Copy link
Member

@fregante fregante commented Oct 21, 2020

Related to #14

This PR currently doesn't include any custom code, it's just the direct result of:

xcrun safari-web-extension-converter distribution/ --project-location safari --app-name 'Refined GitHub' --bundle-identifier 'com.sindresorhus.refinedGitHub'

Next:

@sindresorhus
Copy link
Member

You need to gitignore:

xcuserdata
project.xcworkspace

@yakov116 yakov116 added the meta Related to Refined GitHub itself label Oct 22, 2020
@fregante
Copy link
Member Author

fregante commented Oct 22, 2020

You need to gitignore:

That's what I was hoping to find out with this PR 🎉


Also, initial Safari compatibility achieved

Screen Shot 2020-10-21 at 23 05 38

Options are preserved too:

@fregante fregante added safari Related to Safari only and removed meta Related to Refined GitHub itself labels Oct 22, 2020
@fregante
Copy link
Member Author

Firefox vs Safari vs Chrome

@fregante fregante marked this pull request as ready for review October 22, 2020 05:51
@fregante
Copy link
Member Author

fregante commented Oct 22, 2020

How to test

  1. Install XCode 12 and Safari 14

  2. You probably need to enable Allow Unsigned Extensions in Safari’s Develop menu.

  3. Run:

    npm run build
    npm run pack:safari
    npm run start:safari
  4. I also had to click Edit websites under Safari Preferences -> Extensions -> Refined GitHub and manually allow GitHub.com.

Questions

  1. About the number 4 above: Is this because it's unsigned? Does it happen for you as well? How can we avoid this?

Review

All the new files are autogenerated. Suggestions welcome, even if it's just to rename files.

Next

I think this PR won't need substantial work, but the next steps in new PRs will be:

  • @sindresorhus sign the extension (?)
  • @sindresorhus do you want to publish an initial version manually?
  • Set up Fastlane
  • Improve icons
  • Meta: Find out if there's a way to auto-pack the extension while developing
  • Enterprise: Look into Enterprise support

@fregante
Copy link
Member Author

fregante commented Oct 22, 2020

Regarding point 4: I think there's no way around it. permissions is essentially ignored and always acts as optional_permissions, the user has to enable the extension. More information from ~10:00 to ~15:00 in the video: https://developer.apple.com/videos/play/wwdc2020/10665/

Enterprise support in Safari requires some changes:

@lautis
Copy link
Contributor

lautis commented Oct 22, 2020

You already use the nolookbehind variants, so that's fixed 👍

Another patch I have for Safari is the following CSS:

.rgh-parse-backticks {
  display: inline-block;
}

This fixes line-wrapping of code blocks in issues list.

@fregante
Copy link
Member Author

fregante commented Oct 22, 2020

Another patch I have for Safari is the following CSS:

Thanks for making me notice that. It looks like a Safari bug. inline-block will prevent wrapping though, but it looks like moving the sr-only elements outside rgh-parse-backticks fixes it:

https://github.com/sindresorhus/refined-github/blob/a38244e63e108b141745a80f0264ebd1ff6740b0/source/github-helpers/parse-backticks.tsx#L11-L15

<span className="sr-only">`</span> 
<code className="rgh-parse-backticks">{text.trim()}</code> 
<span className="sr-only">`</span> 

@fregante fregante changed the title Add initial Safari wrapper Add Safari build Oct 23, 2020
@fregante fregante changed the title Add Safari build Add unpublished Safari build Oct 25, 2020
@fregante fregante merged commit a38bf75 into master Oct 25, 2020
@fregante fregante deleted the safari branch October 25, 2020 00:54
@fregante fregante mentioned this pull request Feb 3, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted safari Related to Safari only
Development

Successfully merging this pull request may close these issues.

4 participants