Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

feat: implement inpage navigation - INNO-1555 #104

Merged
merged 7 commits into from
Jun 25, 2019
Merged

feat: implement inpage navigation - INNO-1555 #104

merged 7 commits into from
Jun 25, 2019

Conversation

haringsrob
Copy link
Contributor

@haringsrob haringsrob commented Jun 24, 2019

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • I have put the vanilla component as devDependencies
  • I have put the specs package as devDependencies
  • I have added the components directly used in the twig file (with include or embed) as dependencies
  • My component is listed in @ecl-twig/ec-components's dependencies
  • My variables naming follow the guidelines (snake case for twig)
  • I have provided tests
  • I have provided documentation (for the "notes" tab)
  • If my local yarn.lock contains changes, I have committed it
  • I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

@haringsrob haringsrob changed the title wip: INNO-1555 Inpage navigation feat: INNO-1555 Inpage navigation Jun 24, 2019
@haringsrob haringsrob added the pr: review needed Use this label to show that your PR needs to be review label Jun 24, 2019
@haringsrob haringsrob changed the title feat: INNO-1555 Inpage navigation feat: Inpage navigation - INNO-1555 Jun 24, 2019
@yhuard yhuard self-assigned this Jun 24, 2019
@yhuard yhuard added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Jun 24, 2019
@yhuard yhuard changed the title feat: Inpage navigation - INNO-1555 feat: implement inpage navigation - INNO-1555 Jun 24, 2019
Copy link
Contributor

@yhuard yhuard left a comment

Choose a reason for hiding this comment

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

I've spotted some small things to change here and there but overall it's great!

Is it me or https://deploy-preview-104--ecl-twig.netlify.com/ec/?path=/story/components-inpage-navigation--default doesn't feel smooth? When I'm scrolling, I have the impression that it's lagging...

@yhuard yhuard removed their assignment Jun 24, 2019
@haringsrob haringsrob added pr: wip Mark the PR as WIP and removed pr: modification needed labels Jun 24, 2019
@haringsrob
Copy link
Contributor Author

I made the changes requested.

@haringsrob haringsrob added pr: review needed Use this label to show that your PR needs to be review and removed pr: wip Mark the PR as WIP labels Jun 24, 2019
@haringsrob
Copy link
Contributor Author

I dont really notice any "lag" it does skip items when you scroll fast.

@yhuard yhuard added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Jun 25, 2019
@yhuard yhuard self-assigned this Jun 25, 2019
@yhuard yhuard removed their assignment Jun 25, 2019
Copy link
Contributor

@yhuard yhuard left a comment

Choose a reason for hiding this comment

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

Great work!

@yhuard yhuard merged commit 0955c00 into v2.7-dev Jun 25, 2019
@yhuard yhuard deleted the INNO-1555 branch June 25, 2019 06:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants