Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Added a button to link docs to appPage #314

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Added a button to link docs to appPage #314

merged 4 commits into from
Mar 8, 2023

Conversation

harshit-002
Copy link
Contributor

@harshit-002 harshit-002 commented Mar 6, 2023 •
edited
Loading

Fixes Issue

Closes #313

Changes proposed

Add a button that on click redirects to appPage in the docs section of the website.

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

image

Note to reviewers

Cahllagerfeld
Cahllagerfeld reviewed Mar 6, 2023
View reviewed changes
Copy link
Member

@Cahllagerfeld Cahllagerfeld left a comment

Choose a reason for hiding this comment

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

A fix for the failing test is in main. So when merging main into this, the tests should pass again

src/routes/docs/+layout.svelte Outdated
@@ -12,6 +12,12 @@
</div>
<div class="w-full border-gray-300 dark:border-gray-600 lg:w-3/5 lg:border-x lg:px-8">
<slot />
<div class="flex" style=" justify-content: space-around; margin-top:5%;">
<a
class="default-transition text-skin-text-highlight hover:underline hover:decoration-inherit"
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add the link to /app to the topbar, what do you think?

Copy link
Contributor Author

@harshit-002 harshit-002 Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@Cahllagerfeld yeah, then i think i should name it just "AppPage" and place it in topbar, or any other name you suggest.
image

@harshit-002
Copy link
Contributor Author

A fix for the failing test is in main. So when merging main into this, the tests should pass again

sorry, can you explain a bit more why the test failed, i'm a newbie.

@Cahllagerfeld
Copy link
Member

To be honest, I'm not 100% sure because the tests weren't changed. I think it relates to a self-closing popup, but its just a guess

Cahllagerfeld
Cahllagerfeld reviewed Mar 7, 2023
View reviewed changes
src/lib/components/header.svelte Outdated
const navItems = [{ name: 'Docs', path: '/docs' }];
const navItems = [
{ name: 'Docs', path: '/docs' },
{ name: 'AppPage', path: '/app' },
Copy link
Member

Choose a reason for hiding this comment

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

Shall we just call it App or something? I'm not sure about this camle case here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall we just call it App or something? I'm not sure about this camle case here?

yeah,camel case doesn't seem fine👍

Cahllagerfeld
Cahllagerfeld approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@Cahllagerfeld Cahllagerfeld left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@Cahllagerfeld Cahllagerfeld merged commit 9f7f11a into EddieHubCommunity:main Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Cahllagerfeld Cahllagerfeld Cahllagerfeld approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Button to go back to app from Docs page
2 participants
@harshit-002 @Cahllagerfeld