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

refactor($theme-default): extract page components #1427

Merged
merged 5 commits into from
Sep 7, 2019

Conversation

mathieutu
Copy link
Contributor

Summary
Hi,
This PR extract the Edit and Nav components from default theme Page component.
It allows us to replace easily theses components when extending the template, by replacing only the targeted component and not the whole page.

There is no any change visually speaking.

Thanks for your work!
Matt'

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

The PR fulfills these requirements:

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

@mathieutu mathieutu force-pushed the extract-page-components branch from f933be4 to ec64dac Compare March 11, 2019 18:13
@mathieutu mathieutu changed the title Extract page components refactor($theme-default): Extract page components Mar 11, 2019
@mathieutu mathieutu changed the title refactor($theme-default): Extract page components refactor($theme-default): extract page components Mar 11, 2019
@ulivz
Copy link
Member

ulivz commented Mar 12, 2019

//cc @shigma

Copy link
Collaborator

@shigma shigma left a comment

Choose a reason for hiding this comment

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

LGTM

@mathieutu
Copy link
Contributor Author

(I think the more we'll split templates in small components the more it'll be easy to use and overwrite them.)

@mathieutu
Copy link
Contributor Author

Maybe could we merge it, then?

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

I know it's been a while since you create the pr @mathieutu i will have a look soon. It look great can you just please resolve the conflict please ?

We will need to update the doc part too about it

@flozero flozero self-assigned this Sep 5, 2019
@flozero flozero added complexity: easy Easy complexity effort: low Around a day or less good first issue Good for newcomers status: core team assigned Core team member assigned topic: theme Relates to VuePress theme type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@kefranabg kefranabg removed the good first issue Good for newcomers label Sep 6, 2019
@flozero
Copy link
Collaborator

flozero commented Sep 7, 2019

Before approve it i have to check the theme override on it if it's working or not but it look good

@flozero flozero merged commit 6b5a002 into vuejs:master Sep 7, 2019
@vue-bot
Copy link

vue-bot commented Sep 7, 2019

Hey @mathieutu, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

ulivz pushed a commit that referenced this pull request Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: easy Easy complexity effort: low Around a day or less status: core team assigned Core team member assigned topic: theme Relates to VuePress theme type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants