Skip to content

Fixes public apps page#2422

Merged
pumfleet merged 14 commits intomainfrom
fixes/public-apps-page
Apr 11, 2022
Merged

Fixes public apps page#2422
pumfleet merged 14 commits intomainfrom
fixes/public-apps-page

Conversation

@zomars
Copy link
Copy Markdown
Contributor

@zomars zomars commented Apr 7, 2022

What does this PR do?

Fixes #2420

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • Logout if you're logged in
  • Go to /apps
  • You should be able to see the apps page

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 7, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

docs – ./apps/docs

🔍 Inspect: https://vercel.com/cal/docs/GqAjVeV23XNvfymnJMdenvgjZt8S
✅ Preview: https://docs-git-fixes-public-apps-page-cal.vercel.app

[Deployment for b7c18a1 canceled]

calendso – ./apps/web

🔍 Inspect: https://vercel.com/cal/calendso/DaArsG6zKrqhFUrmpAa1nFGCCRRJ
✅ Preview: https://calendso-git-fixes-public-apps-page-cal.vercel.app

@zomars zomars requested review from a team and PeerRich April 7, 2022 18:15
@zomars zomars added ♻️ autoupdate tells kodiak to keep this branch up-to-date automerge labels Apr 7, 2022
@vercel vercel Bot temporarily deployed to Preview – docs April 7, 2022 18:16 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 7, 2022 18:21 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 7, 2022 18:24 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 7, 2022 18:29 Inactive
Copy link
Copy Markdown
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

LGTM

const { data: session, status } = useSession();
const loading = status === "loading";
const router = useRouter();
const shouldDisplayUnauthed = router.pathname.startsWith("/apps");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldDisplayUnauthorised or shouldDisplayUnauthorized

Copy link
Copy Markdown
Contributor

@alannnc alannnc left a comment

Choose a reason for hiding this comment

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

Just a const name update before merging

@vercel vercel Bot temporarily deployed to Preview – calendso April 8, 2022 05:35 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 8, 2022 11:52 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 8, 2022 11:52 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 8, 2022 16:52 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 8, 2022 16:52 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 8, 2022 17:01 Inactive
const { data: session, status } = useSession();
const loading = status === "loading";
const router = useRouter();
const shouldDisplayUnauthed = router.pathname.startsWith("/apps");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead of hard-coding/apps we can introduce a prop on Shell to determine whether it should render when unauth or not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Proposed #2437 for your reference

Copy link
Copy Markdown
Contributor Author

@zomars zomars Apr 8, 2022

Choose a reason for hiding this comment

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

As for now we don't plan to add any more public routes. We can do that when that changes.

@vercel vercel Bot temporarily deployed to Preview – docs April 9, 2022 14:41 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 9, 2022 14:41 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 9, 2022 22:36 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 9, 2022 22:36 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 10, 2022 16:00 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 10, 2022 16:00 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 10, 2022 16:21 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 10, 2022 16:21 Inactive
@vercel vercel Bot temporarily deployed to Preview – calendso April 10, 2022 17:21 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 10, 2022 17:21 Inactive
@pumfleet pumfleet merged commit 81f3e82 into main Apr 11, 2022
@pumfleet pumfleet deleted the fixes/public-apps-page branch April 11, 2022 14:21
pumfleet added a commit that referenced this pull request Apr 11, 2022
pumfleet pushed a commit that referenced this pull request Apr 11, 2022
zomars added a commit that referenced this pull request Apr 11, 2022
zomars added a commit that referenced this pull request Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge ♻️ autoupdate tells kodiak to keep this branch up-to-date

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cal.com/apps is blank for unauthed users

5 participants