brainhubeu / react-carousel

A pure extendable React carousel, powered by Brainhub (craftsmen who ❤️ JS)
https://brainhub.eu/
MIT License
1.07k stars 164 forks source link

Fix/pr 536 my mooc fix limit reachable slide #537

Open piotr-s-brainhub opened 4 years ago

piotr-s-brainhub commented 4 years ago

Deployed to https://beghp.github.io/gh-pages-rc-9
Fixes #501
and possibly #298

- [x] an issue linked to the PR


piotr-s-brainhub commented 4 years ago

@Romcol

Thanks for your fix but it doesn't work correctly. For the first time, when reaching the last slide, it moves immediately without doing an animation.

Video: https://recordit.co/WqYMrNNTtg

Romcol commented 4 years ago

Hi @piotr-s-brainhub .

I'm sorry but the issue is already present in the master :/ See https://github.com/brainhubeu/react-carousel/pull/538. This is not due to my PR I regret.

Cheers

piotr-s-brainhub commented 4 years ago

@Romcol

I guess you mean another issue than #538 (it's a simple PR updating the text in docs).

Romcol commented 4 years ago

@piotr-s-brainhub

I don't know which PR has introduced that bug. I'm just saying that it was present in the last PR that was merged :)

piotr-s-brainhub commented 4 years ago

@Romcol

I opened a PR https://github.com/brainhubeu/react-carousel/pull/541 with only your commit but it has the same bug.

Romcol commented 4 years ago

@piotr-s-brainhub

Hum, ok :smile: The github page I was looking at had been overwritten so it got me confused, sorry. You can delete that. I'll check my PR

Romcol commented 4 years ago

@piotr-s-brainhub

I have trouble reproducing the error. When I run npm run start-test-app it fails, I get UNHANDLED REJECTION Menu.pages provided incorrect OutputType: 'Object({ __esModule: true, GraphQLJSON: JSON, default: JSON, GraphQLJSONObject: JSONObject })'.

Do you have any ideas? Cheers

humbak commented 4 years ago

Hi, I working on smart scrolling and smarter dots. My story resolves also this issue. Please for a while of patient. Btw. in this case @Romcol you forgot about dots, you still will render the same number of them.

I have also feedback about the issue which @piotr-s-brainhub mentions. It's not connected to that story. The actual master behaves the same when is reaching a disabled arrow in the first click. Please do the same test but for 2 items in the carousel, then you will see. 2 items on master

Romcol commented 4 years ago

Hi @humbak,

Thank you for your observations. Indeed, I don't think the animation problem in due to my PR, thank you for backing that up. @piotr-s-brainhub

For dots, I don't know what should be done. I think it's still better than the master. Anyway, we can wait for sure for your story :)

brainhub-devops commented 4 years ago
Warnings
:warning: MODULE : node_modules/uglify-js/node_modules/wordwrap | LICENSE : MIT* | LICENSE_FILE : node_modules/uglify-js/node_modules/wordwrap/README.markdown | REPOSITORY: https://github.com/substack/node-wordwrap | PUBLISHER : James Halliday | EMAIL : mail@substack.net | URL : http://substack.net
:warning: MODULE : node_modules/vargs | LICENSE : MIT* | LICENSE_FILE : node_modules/vargs/LICENSE | REPOSITORY: undefined | PUBLISHER : Alexis Sellier | EMAIL : self@cloudhead.net | URL : undefined
:warning: MODULE : node_modules/gemini-coverage/node_modules/uglify-js | LICENSE : BSD* | LICENSE_FILE : node_modules/gemini-coverage/node_modules/uglify-js/LICENSE | REPOSITORY: https://github.com/mishoo/UglifyJS2 | PUBLISHER : undefined | EMAIL : undefined | URL : undefined
:warning: MODULE : node_modules/trim | LICENSE : MIT* | LICENSE_FILE : node_modules/trim/Readme.md | REPOSITORY: undefined | PUBLISHER : TJ Holowaychuk | EMAIL : tj@vision-media.ca | URL : undefined
:warning: MODULE : node_modules/istanbul/node_modules/source-map | LICENSE : BSD | LICENSE_FILE : node_modules/istanbul/node_modules/source-map/LICENSE | REPOSITORY: https://github.com/mozilla/source-map | PUBLISHER : Nick Fitzgerald | EMAIL : nfitzgerald@mozilla.com | URL : undefined
:warning: MODULE : node_modules/gemini-coverage/node_modules/source-map | LICENSE : BSD | LICENSE_FILE : node_modules/gemini-coverage/node_modules/source-map/LICENSE | REPOSITORY: https://github.com/mozilla/source-map | PUBLISHER : Nick Fitzgerald | EMAIL : nfitzgerald@mozilla.com | URL : undefined
:warning: MODULE : node_modules/optimist | LICENSE : MIT* | LICENSE_FILE : node_modules/optimist/LICENSE | REPOSITORY: https://github.com/substack/node-optimist | PUBLISHER : James Halliday | EMAIL : mail@substack.net | URL : http://substack.net
:warning: MODULE : node_modules/map-stream | LICENSE : Custom: https://github.com/dominictarr/event-stream | LICENSE_FILE : node_modules/map-stream/LICENCE | REPOSITORY: https://github.com/dominictarr/map-stream | PUBLISHER : Dominic Tarr | EMAIL : dominic.tarr@gmail.com | URL : http://dominictarr.com
:warning: MODULE : node_modules/jsonify | LICENSE : Public Domain | LICENSE_FILE : node_modules/jsonify/README.markdown | REPOSITORY: https://github.com/substack/jsonify | PUBLISHER : Douglas Crockford | EMAIL : undefined | URL : http://crockford.com/
:warning: MODULE : node_modules/json-schema | LICENSE : AFLv2.1,BSD | LICENSE_FILE : node_modules/json-schema/README.md | REPOSITORY: https://github.com/kriszyp/json-schema | PUBLISHER : Kris Zyp | EMAIL : undefined | URL : undefined
:warning: MODULE : node_modules/indexof | LICENSE : MIT* | LICENSE_FILE : node_modules/indexof/Readme.md | REPOSITORY: undefined | PUBLISHER : undefined | EMAIL : undefined | URL : undefined
:warning: MODULE : node_modules/html | LICENSE : BSD* | LICENSE_FILE : node_modules/html/LICENSE | REPOSITORY: https://github.com/maxogden/commonjs-html-prettyprinter | PUBLISHER : Max Ogden | EMAIL : max@maxogden.com | URL : http://maxogden.com
:warning: MODULE : node_modules/glob-to-regexp | LICENSE : BSD* | LICENSE_FILE : node_modules/glob-to-regexp/README.md | REPOSITORY: https://github.com/fitzgen/glob-to-regexp | PUBLISHER : Nick Fitzgerald | EMAIL : fitzgen@gmail.com | URL : undefined
:warning: MODULE : node_modules/uglifyify/node_modules/extend | LICENSE : MIT* | LICENSE_FILE : node_modules/uglifyify/node_modules/extend/LICENSE | REPOSITORY: https://github.com/justmoon/node-extend | PUBLISHER : Stefan Thomas | EMAIL : justmoon@members.fsf.org | URL : http://www.justmoon.net
:warning: MODULE : node_modules/istanbul/node_modules/estraverse | LICENSE : BSD | LICENSE_FILE : node_modules/istanbul/node_modules/estraverse/LICENSE.BSD | REPOSITORY: https://github.com/estools/estraverse | PUBLISHER : undefined | EMAIL : undefined | URL : undefined
:warning: MODULE : node_modules/domutils | LICENSE : BSD* | LICENSE_FILE : node_modules/domutils/LICENSE | REPOSITORY: https://github.com/FB55/domutils | PUBLISHER : Felix Boehm | EMAIL : me@feedic.com | URL : undefined
:warning: MODULE : node_modules/eslint-plugin-import/node_modules/doctrine | LICENSE : BSD | LICENSE_FILE : node_modules/eslint-plugin-import/node_modules/doctrine/LICENSE.BSD | REPOSITORY: https://github.com/eslint/doctrine | PUBLISHER : undefined | EMAIL : undefined | URL : undefined
:warning: MODULE : node_modules/css-select | LICENSE : BSD* | LICENSE_FILE : node_modules/css-select/LICENSE | REPOSITORY: https://github.com/fb55/css-select | PUBLISHER : Felix Boehm | EMAIL : me@feedic.com | URL : undefined
:warning: MODULE : node_modules/parse-color/node_modules/color-convert | LICENSE : MIT* | LICENSE_FILE : node_modules/parse-color/node_modules/color-convert/LICENSE | REPOSITORY: https://github.com/harthur/color-convert | PUBLISHER : Heather Arthur | EMAIL : fayearthur@gmail.com | URL : undefined
:warning: MODULE : node_modules/async-foreach | LICENSE : MIT* | LICENSE_FILE : node_modules/async-foreach/LICENSE-MIT | REPOSITORY: https://github.com/cowboy/javascript-sync-async-foreach | PUBLISHER : "Cowboy" Ben Alman | EMAIL : undefined | URL : http://benalman.com/

Generated by :no_entry_sign: dangerJS against ed04b2beea5d4db83104b7108014825ea6f70d97

ScriptedAlchemy commented 4 years ago

@brainhubeu @piotr-s-brainhub @RobertHebel is there anything we can do to get this merged. Currently experiencing UX issues without it and would prefer an npm @next tag over forking and publishing a temporary copy.

Also happy to assist in maintaining this codebase as we are dependent on its ability to function