feat: Support pagination in issue-list screen and pull-list screen #856
Conversation
| @@ -23,68 +20,6 @@ describe('Issuer Reducer', () => { | |||
| expect(issueReducer()).toEqual(initialState); | |||
| }); | |||
|
|
|||
| describe('GET_ISSUE_COMMENTS', () => { | |||
There was a problem hiding this comment.
Thanks for cleaning this up 🚀
| @@ -12,6 +12,7 @@ export const ACTIVITY_WATCH_REPO = createActionSet('ACTIVITY_WATCH_REPO'); | |||
| export const ACTIVITY_UNWATCH_REPO = createActionSet('ACTIVITY_UNWATCH_REPO'); | |||
| export const SEARCH_REPOS = createPaginationActionSet('SEARCH_REPOS'); | |||
| export const SEARCH_USERS = createPaginationActionSet('SEARCH_USERS'); | |||
| export const SEARCH_ISSUES = createPaginationActionSet('SEARCH_ISSUES'); | |||
| @@ -48,8 +48,7 @@ export const repoSchema = new schema.Entity( | |||
| 'repos', | |||
| {}, | |||
| { | |||
| idAttribute: repo => | |||
| (repo.full_name ? repo.full_name : repo.name).toLowerCase(), | |||
| idAttribute: repo => (repo.full_name ? repo.full_name : repo.name), | |||
There was a problem hiding this comment.
I'm assuming there's no harm to just keeping the repo name as is (without lowercase) as the idAttribute right? (definitely better this way but just want to double check)
There was a problem hiding this comment.
I remember some edge cases where the case was a problem, but definitely can't tell what it was now.
Let's drop the lowercase and see what happens :)
There was a problem hiding this comment.
@housseindjirdeh @machour Thanks for your concerns. @machour was inspired from redux examples. It reads user input as keyword to fetch Github data. And when finding entities, it also converts the key to lower cases, see link. Because Github APIs accept case-insensitive arguments and return case-sensitive data, as I commented in #826 (comment).
I think our use cases are a bit different. We always use case-sensitive ids. So we should stop converting entity ids to lower cases.
| mapping keys | entities ids | result | |
|---|---|---|---|
| redux examples | always lower cases | always lower cases | no problem |
| our old codes | not sure | always lower cases | can't find entity |
| our new codes | always case-sensitive | always case-sensitive | no problem |
There was a problem hiding this comment.
That clears things up nicely, thank you makes perfect sense :)
Screenshots
Description
Sadly, the ability to show different colors between closed and merged pull requests, see #852, is broken. Because the search API always returns state
closed. I will open an issue to record it and hope someone can implement later.