[angular-apollo-tailwind] Audit app for any architectural improvements and features that are incomplete

[Atrophius]

Acceptance

As either a write up in Google Docs or as comments on this issue, please provide us with:

• Any bugs you find clicking through the app
• Any architectural issues you see with components, their styles, or general cleanup required

Notes

See #489 for a good example

[ktrz]

Angular Apollo Trailwind

Setup issues

• [ ] README mentions using npm but yarn should be preferred if we are using yarn.lock file
• [ ] no mention in README to run yarn generate
• [ ] .env.example doesn't provide a proper initial config. Setup that worked for me

  GITHUB_GRAPHQL_URL=https://api.github.com/graphql
  OAUTH_API_URL=https://api.starter.dev
  CLIENT_REDIRECT_URL=http://localhost:4200/redirect
  PRODUCTION=false

Bugs

• [ ] same as https://github.com/thisdot/starter.dev-github-showcases/issues/489#issuecomment-1192909847
• pull requests / issues view
  • [ ] after selecting closed PRs filter it's not possible to select 'open' issues (had to select other filter and then click clear filters button)

* it works properly on `Issues`

• [ ] PR title is not clickable (should probably at least link to an original GitHub PR page)
• [ ] label filter only includes labels that are visible on the currently loaded PRs (from current page)
  • all labels should be visible
• [ ] clicking on x in label selector selects the first label instead of clearing the filter (or errors in console if no labels are available)

• [ ] there is no way to unselect the label filter by clicking a given label again
• [ ] label selector doesn't support multiple selections
• [ ] skeleton loader differs between Pull requests and Issues page.

* It should be the same on both sites. Possibly displaying skeleton for header as well but without 0 shown for open/closed counter

• [ ] Filters on PRs don't include Milestones (but they do on issues page)
  • filters should be aligned
• [ ] Readme link in the About section should scroll to the README section

• Nice to have:
  • [ ] display more information in the repo about section
    • stars, watchers, forks counters with links

* [ ] navigating to a page that doesn't have any data should show 404 page. Currently, it will try loading eg. a user and display a graphql error

Code

Code review

• [ ] the directory structure seems a little inconsistent
  • root level
  • technical directories are mixed with the feature directories (eg. components, utils mixed with home, profile, issues)
    • I would suggest keeping the root level structure focused on features and if eg. common components are needed extracting those into a separate library (or create a shared root directory)
• inconsistencies in how component's store is used
  • [ ] pull requests use single store that also manages filters, however issues have 2 separate stores
  • the approach should be unified
  • [ ] component's state seems to be designed in a way that doesn't support easy reactivity eg.

    export class IssuesComponent implements OnInit {
    setLabel(label: string) {
    this.reposFilterStore.setLabel(label);
    this.issuesStore.getIssues$(this.filterState$);
    }
    }

    Currently, every time we change anything in the filter state we need to manually call this.issuesStore.getIssues$(this.filterState$). It should be a responsibility of the store to manage that IMO. Similar thing happens in pull requests logic

  • [ ] Repos component on the other hand doesn't use the component's store at all. I think it would be good to extract the state logic into repo's own component state to be consistent (and also hide GraphQL implementation details)
• [ ] modules are too big
  • eg. ProfileModule and ReposModule declare respectively 13 and 12 components. Modules should be kept small so that we don't need to maintain a large collection of imports that might be used only in a couple of components. We should probably start moving towards using standalone components now.
• [ ] profile directory can probably be split into user and org
  • ProfileResolver uses ActivatedRouteSnapshot.url to resolve params, but they could use ActivatedRouteSnapshot.paramMap.get('owner') to avoid conditional logic

[Atrophius]

@ktrz Review board for tickets related to this audit: https://github.com/orgs/thisdot/projects/1/views/5

Create tickets for anything that's not accounted for. If coverage is sufficient, you can close this ticket.