Skip to content

Redsign/new header #5328

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

Closed
wants to merge 59 commits into from
Closed

Redsign/new header #5328

wants to merge 59 commits into from

Conversation

u-rogel
Copy link
Contributor

@u-rogel u-rogel commented Apr 24, 2023

@ovflowd it is way to early to be merged but I have some questions:

  1. In here - _app.mdx we import old styles, can we change to the new ones?
  2. This jsonp - components.header.links.learn, is missing from our i18n schema, do we have a script to add it?
  3. In theme.tsx I needed to comment out this line, otherwise will get an error. Any idea?
    Screenshot from 2023-04-25 00-00-02

AugustinMauroy and others added 30 commits April 15, 2023 10:17
Co-authored-by: Shanmughapriyan S <[email protected]>
Co-authored-by: Michael Esteban <[email protected]>
Co-authored-by: Claudio Wunder <[email protected]>
Migrate AnimatedPlaceholder component from nodejs.dev
and create a new Story.
chore: next lock to versin 13.2.0
Co-authored-by: Claudio Wunder <[email protected]>
Co-authored-by: Teja Sai Sandeep Reddy Konala <[email protected]>
Co-authored-by: Shanmughapriyan S <[email protected]>
Co-authored-by: Michael Esteban <[email protected]>
…mplates (#5294)

Co-authored-by: Shanmughapriyan S <[email protected]>
Co-authored-by: Manish Kumar ⛄ <[email protected]>
…ies (#5319

* chore: optimises tsconfig

* chiore: add missing dependencies

* chore: type storybook constants

* chore: styles moved styles to somewhere else

* chore: add global json type definition

* chore: i18n aria-label instead of sr-only

* chore: added open sans font family and space between imports

* chore: moved styles and fixed styles and updated banner stories

* fix: stylelint rules

* chore: updated tsconfig

* chore: fix tests

* fix: darkmodetoggle test

* chore: stories use index.stories.tsx
* chore: revert pnpm use plain npm

* fix: package.json

* chore: remove warnings and add node_env

* chore: cross-env

* fix: fix turbo pipelines

* chore: only cache certain files

* chore: turbo shouldn't care about coverage outputs

* chore: proper inputs and outputs for pipelines

* chore: do not store some outputs and updated inputs for lint

* chore: added prettier configs

* chore: remove console.info

* chore: updated inputs of all other entries
fix(package.json) lint:fix missing slashes
Co-authored-by: vasanth9 <cheepurupalli.vasanthkumar.com>
Co-authored-by: Shanmughapriyan S <[email protected]>
@u-rogel
Copy link
Contributor Author

u-rogel commented May 2, 2023

@ovflowd @AugustinMauroy ready for another review I would say.

@vercel vercel bot temporarily deployed to Preview – nodejs-org May 2, 2023 20:21 Inactive
@vercel vercel bot temporarily deployed to Preview – nodejs-org-stories May 2, 2023 20:22 Inactive
min-height: calc(1 * var(--nav-height));
padding: 0 var(--space-12);
}
.startWrapper {
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
.startWrapper {
.startWrapper {

@vercel vercel bot temporarily deployed to Preview – nodejs-org-stories May 2, 2023 20:27 Inactive
@vercel vercel bot temporarily deployed to Preview – nodejs-org May 2, 2023 20:29 Inactive
@ovflowd
Copy link
Member

ovflowd commented May 2, 2023

@u-rogel code looks nice! But tests failing, probably you need to mock some more stuff!

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

Nice work 🚀 !

@@ -0,0 +1,9 @@
import Header from './index';
Copy link
Member

Choose a reason for hiding this comment

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

We should create stories to see active link.
Useful link: nextjs router storybook

@ovflowd ovflowd force-pushed the major/website-redesign branch from 7ddfa8d to f2a3ac4 Compare May 3, 2023 23:05
@ovflowd ovflowd deleted the branch nodejs:major/website-redesign May 4, 2023 13:23
@ovflowd ovflowd closed this May 4, 2023
@ovflowd
Copy link
Member

ovflowd commented May 4, 2023

This PR got accidentally closed because the branch got deleted. Please feel free to reopen the PR.

@ovflowd ovflowd reopened this May 4, 2023
@AugustinMauroy
Copy link
Member

Hey 👋 @u-rogel!
Any news ?

@u-rogel
Copy link
Contributor Author

u-rogel commented May 7, 2023

Hey 👋 @u-rogel! Any news ?

Hi, sure will submit something later

@vercel vercel bot temporarily deployed to Preview – nodejs-org-stories May 7, 2023 21:23 Inactive
@vercel vercel bot temporarily deployed to Preview – nodejs-org May 7, 2023 21:27 Inactive
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

Not bad 😁!!!
just don't miss to rebase/chery-pick


describe('Tests for Header component', () => {
beforeEach(() => {
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

what is the ts error ?

<Header />
</IntlProvider>
);
expect(container).toMatchSnapshot();
Copy link
Member

Choose a reason for hiding this comment

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

Now we use storybook to create snapshot you should update unit test.

@u-rogel
Copy link
Contributor Author

u-rogel commented May 8, 2023

Not bad 😁!!!
just don't miss to rebase/chery-pick

Will do!

@vercel vercel bot temporarily deployed to Preview – nodejs-org-stories May 8, 2023 21:42 Inactive
@vercel vercel bot temporarily deployed to Preview – nodejs-org May 8, 2023 21:46 Inactive
@u-rogel
Copy link
Contributor Author

u-rogel commented May 8, 2023

@AugustinMauroy I think I got too far from the target branch. Will create a new PR from the current stage.

@u-rogel u-rogel closed this May 8, 2023
@AugustinMauroy
Copy link
Member

Oh ok no problem !

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.