Skip to content

Conversation

@ggazzo
Copy link
Member

@ggazzo ggazzo commented Dec 29, 2021

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@ggazzo ggazzo force-pushed the feat/apple-oauth branch 3 times, most recently from 19f1fbb to 93e2f4a Compare December 29, 2021 22:38
@ggazzo ggazzo requested review from a team January 12, 2022 13:59
@jeanfbrito
Copy link
Contributor

giphy

jeanfbrito
jeanfbrito previously approved these changes Jan 12, 2022
Co-authored-by: Diego Sampaio <chinello@gmail.com>
showButton: false,
secret,
enabled: settings.get('Accounts_OAuth_Apple'),
loginStyle: 'redirect',
Copy link
Member

Choose a reason for hiding this comment

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

should this just use config.loginStyle instead of having duplicated here?

scope: 'name email',
mergeUsers: true,
accessTokenParam: 'access_token',
loginStyle: 'redirect',
Copy link
Member

Choose a reason for hiding this comment

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

should we use 'popup' instead? I remember we discussing how 'redirect' could be bad for the desktop app.

Copy link
Member Author

Choose a reason for hiding this comment

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

I dont think this has any effect since we trigger the action by the apple script
but sure its a little messed

export const AppleOauthButton: FC = () => {
const enabled = useSetting('Accounts_OAuth_Apple');
const absoluteUrl = useAbsoluteUrl();
const appleClientID = useSetting('Accounts_OAuth_Apple_id') || '[CLIENT_ID]';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const appleClientID = useSetting('Accounts_OAuth_Apple_id') || '[CLIENT_ID]';
const appleClientID = useSetting('Accounts_OAuth_Apple_id');

@ggazzo ggazzo merged commit 1441feb into develop Jan 14, 2022
@ggazzo ggazzo deleted the feat/apple-oauth branch January 14, 2022 18:12
gabriellsh added a commit that referenced this pull request Jan 17, 2022
…ove/setup-wizard

* 'develop' of github.com:RocketChat/Rocket.Chat: (176 commits)
  [IMPROVE] Admin page header buttons consistency (#24168)
  i18n: Language update from LingoHub 🤖 on 2022-01-17Z (#24193)
  [FIX] Integration section crashing opening in My Account (#24068)
  [IMPROVE] Rewrite roomNotFound to React Component (#24044)
  Regression: Enable custom emoji on admin custom status page (#24186)
  Chore: Update Meteor to 2.5.3 (#24075)
  [NEW] Apple Login (#24060)
  Chore: Update Apps-Engine to 1.29.2 (#24171)
  feat: enabling emoji on custom status (#24170)
  [FIX] App Framework Enable hanging indefinitely (#24158)
  [FIX] CSV Importer failing to import users (#24090)
  Fix Engagement Dashboard API requests (#24142)
  Language update from LingoHub 🤖 (#24127)
  Chore: Migrate useOutsideClick to fuselage-hooks (#24133)
  Revert "Use fibers to store context"
  Use fibers to store context
  Chore: Include REG_TOKEN in docker-compose (#24123)
  [FIX] Custom Emoji Image preview #24117
  [IMPROVE] Added a Reset Button in the Account Profile Page (#24078)
  Revert: "[IMPROVE] Throw 404 error in invalid endpoints" (#24118)
  ...
@sampaiodiego sampaiodiego mentioned this pull request Jan 29, 2022
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.

4 participants