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

feat(v2): add edit url in post page #2524

Merged
merged 16 commits into from
Apr 5, 2020

Conversation

fanny
Copy link
Contributor

@fanny fanny commented Apr 4, 2020

Motivation

Fix #2501

Add label option to edit page in blog as in doc

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

go to a post page

B0625505-1D5C-4515-9213-8A6BCA2D4EB7_1_105_c

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 4, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 4, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 8e26f99

https://deploy-preview-2524--docusaurus-2.netlify.com

@yangshun yangshun added this to the v2.0.0-alpha.51 milestone Apr 4, 2020
@fanny
Copy link
Contributor Author

fanny commented Apr 4, 2020

@yangshun I didn't found the part on the doc, that we mention about the blog config, to update. Only for the docs. Do you think that would it be nice add?

@fanny fanny changed the title [WIP] Add edit url in post page feat(v2): Add edit url in post page Apr 4, 2020
@fanny fanny marked this pull request as ready for review April 4, 2020 13:12
@fanny fanny requested review from lex111, wgao19 and yangshun as code owners April 4, 2020 13:12
Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

Need tests.

@fanny
Copy link
Contributor Author

fanny commented Apr 4, 2020

Hi @lex111. Do I need to implement jest tests? What type of test do you suggest?

@lex111
Copy link
Contributor

lex111 commented Apr 4, 2020

See test below as an example.

test('docs with editUrl', async () => {
const editUrl =
'https://github.com/facebook/docusaurus/edit/master/website';
const source = path.join('foo', 'baz.md');
const options = {
routeBasePath,
editUrl,
};
const data = await processMetadata({
source,
refDir: docsDir,
context,
options,
env,
});
expect(data).toEqual({
id: 'foo/baz',
permalink: '/docs/foo/baz',
source: path.join('@site', routeBasePath, source),
title: 'baz',
editUrl:
'https://github.com/facebook/docusaurus/edit/master/website/docs/foo/baz.md',
description: '## Images',
});
});


function EditPage({url}) {
return (
<div>
Copy link
Contributor

@lex111 lex111 Apr 4, 2020

Choose a reason for hiding this comment

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

Remove redundant div.

@yangshun yangshun changed the title feat(v2): Add edit url in post page feat(v2): add edit url in post page Apr 5, 2020
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Thank you, this looks really good. But let's not touch the docs theme markup (extracting the utility function is fine, but leave the markup duplicated). We don't want to create too many theme components.

Few more things:

  • Document the new option in website/docs/using-plugins.md under the docs plugin section 😄
  • Add this option into the default templates (can be done in a follow up PR)

@@ -13,6 +13,7 @@ import useDocusaurusContext from '@docusaurus/useDocusaurusContext';
import useBaseUrl from '@docusaurus/useBaseUrl';
import DocPaginator from '@theme/DocPaginator';
import useTOCHighlight from '@theme/hooks/useTOCHighlight';
import EditPage from '@theme/EditPage';
Copy link
Contributor

Choose a reason for hiding this comment

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

Just copy and paste this component, don't make a new component for this edit button. Anyway it should probably be called EditButton if we wanted to make it a component.

@@ -26,12 +29,10 @@ function BlogPostPage(props) {
isBlogPostPage>
<BlogPostContents />
</BlogPostItem>
{(metadata.nextItem || metadata.prevItem) && (
<EditPage url={editUrl} />
Copy link
Contributor

@yangshun yangshun Apr 5, 2020

Choose a reason for hiding this comment

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

Just duplicate the SVG and the <a>

@lex111 lex111 removed this from the v2.0.0-alpha.51 milestone Apr 5, 2020
@yangshun yangshun force-pushed the feat/add-editurl-in-post branch from 30bfe7b to 112f42b Compare April 5, 2020 09:29
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

I made some changes, let's merge this first.

You can make a follow up PR to add showReadingTime and editUrl to the templates 😁

@yangshun yangshun merged commit 5e664a1 into facebook:master Apr 5, 2020
@yangshun yangshun added the pr: new feature This PR adds a new API or behavior. label Apr 5, 2020
@yangshun yangshun added this to the v2.0.0-alpha.51 milestone Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(v2): add edit link to blog posts
5 participants