-
Notifications
You must be signed in to change notification settings - Fork 71
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
Sorting #48
base: master
Are you sure you want to change the base?
Sorting #48
Conversation
I don't think this project is maintained anymore. Would you mind looking into my fork? Maybe send the PR there! I've migrated react-gateway to use the new react hooks! |
Ok. |
@chardskarth Your project doesn't have this bug because Object.keys() sorts keys correctly by default |
children: Object.keys(this._children[name]).sort().map(id => this._children[name][id]) | ||
children: Object.keys(this._children[name]) | ||
.sort((a, b) => this._getChildIndex(a) - this._getChildIndex(b)) | ||
.map(id => this._children[name][id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kseniya57 do you know if the name
even needs to be part of the key; i.e. any reason ln 47 can't just be:
const gatewayId = this._currentId;
That way this could avoid the regex and instead cast it to a number?
I've not looked too deep into your code @chardskarth but if the original react-gateway was to avoid sorting and merely rely on insertion order via |
I'm not sure I'd want to remove I'm thinking I'd rather introduce a new prop and have it used in sorting instead of relying in Maybe I'll look into adding that new prop in my fork. |
Just in case anybody else needs this PR, I went ahead and published it to NPM https://www.npmjs.com/package/@riscarrott/react-gateway |
@richardscarrott @kseniya57 just in case you still need this, I bumped the version to 3.1.1 |
strings sorting works incorrectly one of children has id more than 9, it becames first because it's id starts with 1