Skip to content
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

Combo only UI redesign #2514

Merged
merged 25 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c14d9ce
Design I init
frzyc Oct 16, 2024
a4934cf
preset conditionals, bonusStats
frzyc Oct 16, 2024
9da979c
Merge remote-tracking branch 'origin/master' into combo_only_experiment
frzyc Oct 19, 2024
d7567c2
Merge and fix remote-tracking branch 'origin/master' into combo_only_…
frzyc Oct 21, 2024
a378041
futher fixes
frzyc Oct 21, 2024
f6230c0
cleanup
frzyc Oct 21, 2024
09291a8
add opt UI
frzyc Oct 22, 2024
5475336
Merge remote-tracking branch 'origin/master' into combo_only_experiment
frzyc Oct 23, 2024
77959d7
initial funneling of combo to solver
frzyc Oct 23, 2024
6e96317
Merge remote-tracking branch 'origin/master' into combo_only_experiment
frzyc Oct 24, 2024
4dec2e7
fix comment
frzyc Oct 24, 2024
337c3f9
fix tag data
frzyc Oct 24, 2024
ea5b323
clean up console logs
frzyc Oct 25, 2024
cbdbe7a
remove page-team/s
frzyc Oct 25, 2024
43bbf9c
rename of page-combo/s to page-team/s
frzyc Oct 25, 2024
8d21b8f
more cleanup
frzyc Oct 25, 2024
403808b
create sr-ui-db lib
frzyc Oct 26, 2024
5f78647
speeling + tag comparison
frzyc Oct 26, 2024
c20217b
some updates
frzyc Oct 26, 2024
de5f856
Merge branch 'master' into combo_only_experiment
frzyc Oct 26, 2024
b030d23
fixes and memoization
frzyc Oct 26, 2024
693e951
Merge remote-tracking branch 'origin/master' into combo_only_experiment
frzyc Oct 28, 2024
b746243
rename teammateDatum
frzyc Oct 30, 2024
0e952fd
Merge remote-tracking branch 'origin/master' into combo_only_experiment
frzyc Oct 30, 2024
c6fca35
reshuffle
frzyc Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/sr-frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ScrollTop } from '@genshin-optimizer/common/ui'
import { DatabaseProvider } from '@genshin-optimizer/sr/db-ui'
import '@genshin-optimizer/sr/i18n' // import to load translations
import { theme } from '@genshin-optimizer/sr/theme'
import { DatabaseProvider } from '@genshin-optimizer/sr/ui'
import {
Box,
Container,
Expand Down
2 changes: 1 addition & 1 deletion apps/sr-frontend/src/app/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { useDatabaseTally } from '@genshin-optimizer/common/database-ui'
import { Tally } from '@genshin-optimizer/common/ui'
import { useDatabaseContext } from '@genshin-optimizer/sr/db-ui'
frzyc marked this conversation as resolved.
Show resolved Hide resolved
frzyc marked this conversation as resolved.
Show resolved Hide resolved
import {
CharacterIcon,
LightConeIcon,
RelicIcon,
TeamsIcon,
} from '@genshin-optimizer/sr/svgicons'
import { useDatabaseContext } from '@genshin-optimizer/sr/ui'
import { Settings } from '@mui/icons-material'
import MenuIcon from '@mui/icons-material/Menu'
import {
Expand Down
20 changes: 18 additions & 2 deletions libs/common/util/src/lib/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ export const getObjectKeysRecursive = (obj: unknown): string[] =>

export function deepFreeze<T>(obj: T, layers = 5): T {
if (layers === 0) return obj
if (typeof obj === 'object')
Object.values(Object.freeze(obj)).forEach((o) => deepFreeze(o, layers--))
if (obj && typeof obj === 'object')
Object.values(Object.freeze(obj)).forEach((o) => deepFreeze(o, layers - 1))
return obj
}

Expand Down Expand Up @@ -215,3 +215,19 @@ export function reverseMap<K extends string, V extends string>(
Object.entries(obj).map(([k, v]) => [v, k])
) as Record<V, K>
}

export function shallowCompareObj<T extends Record<string, any>>(
obj1: T,
obj2: T
) {
const keys1 = Object.keys(obj1)
const keys2 = Object.keys(obj2)

// Check if the objects have the same number of keys
if (keys1.length !== keys2.length) return false

// Check if all keys and their values are the same
for (const key of keys1) if (obj1[key] !== obj2[key]) return false
nguyentvan7 marked this conversation as resolved.
Show resolved Hide resolved

return true
}
13 changes: 13 additions & 0 deletions libs/sr/db-ui/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"presets": [
[
"@nx/react/babel",
{
"runtime": "automatic",
"useBuiltIns": "usage",
"importSource": "@emotion/react"
}
]
],
"plugins": ["@emotion/babel-plugin"]
}
18 changes: 18 additions & 0 deletions libs/sr/db-ui/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["plugin:@nx/react", "../../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
7 changes: 7 additions & 0 deletions libs/sr/db-ui/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# sr-db-ui

This library was generated with [Nx](https://nx.dev).

## Running unit tests

Run `nx test sr-db-ui` to execute the unit tests via [Jest](https://jestjs.io).
9 changes: 9 additions & 0 deletions libs/sr/db-ui/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "sr-db-ui",
"$schema": "../../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/sr/db-ui/src",
"projectType": "library",
"tags": [],
"// targets": "to see all targets run: nx show project sr-db-ui --web",
"targets": {}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import type { ICachedCharacter } from '@genshin-optimizer/sr/db'
import { createContext, useContext } from 'react'

export type CharacterContextObj = {
character: ICachedCharacter | undefined
}

export const CharacterContext = createContext({} as CharacterContextObj)
export const CharacterContext = createContext(
undefined as ICachedCharacter | undefined
)

export function useCharacterContext() {
return useContext(CharacterContext)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export * from './CharacterContext'
export * from './DatabaseContext'
export * from './LoadoutContext'
5 changes: 5 additions & 0 deletions libs/sr/db-ui/src/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export * from './useBuild'
export * from './useCharacter'
export * from './useEquippedRelics'
export * from './useLightCone'
export * from './useTeam'
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useDataManagerBase } from '@genshin-optimizer/common/database-ui'
import { useDatabaseContext } from '../Context'
import { useDatabaseContext } from '../context'
export function useBuild(buildId: string | undefined) {
const { database } = useDatabaseContext()
return useDataManagerBase(database.builds, buildId ?? '')
Expand Down
8 changes: 8 additions & 0 deletions libs/sr/db-ui/src/hooks/useCharacter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { useDataManagerBase } from '@genshin-optimizer/common/database-ui'
import type { CharacterKey } from '@genshin-optimizer/sr/consts'
import { useDatabaseContext } from '../context'

export function useCharacter(characterKey: CharacterKey | '' | undefined) {
const { database } = useDatabaseContext()
return useDataManagerBase(database.chars, characterKey as CharacterKey)
}
Comment on lines +5 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add type safety and error handling.

The current implementation type-casts potentially invalid values ('' or undefined) to CharacterKey without validation, which could lead to runtime errors.

Consider adding validation and proper error handling:

 export function useCharacter(characterKey: CharacterKey | '' | undefined) {
   const { database } = useDatabaseContext()
-  return useDataManagerBase(database.chars, characterKey as CharacterKey)
+  if (!characterKey) return undefined
+  return useDataManagerBase(database.chars, characterKey)
 }

Committable suggestion was skipped due to low confidence.

frzyc marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from '@genshin-optimizer/sr/consts'
import type { ICachedCharacter, ICachedRelic } from '@genshin-optimizer/sr/db'
import { useEffect, useMemo, useState } from 'react'
import { useDatabaseContext } from '../Context/DatabaseContext'
import { useDatabaseContext } from '../context'

export function useEquippedRelics(
equippedRelics: ICachedCharacter['equippedRelics'] | undefined
Expand Down
7 changes: 7 additions & 0 deletions libs/sr/db-ui/src/hooks/useLightCone.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { useDataManagerBase } from '@genshin-optimizer/common/database-ui'
import { useDatabaseContext } from '../context'

export function useLightCone(lightConeId: string | undefined) {
const { database } = useDatabaseContext()
return useDataManagerBase(database.lightCones, lightConeId ?? '')
}
Comment on lines +4 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for the database hooks

The verification reveals:

  • No direct tests for useLightCone hook
  • No tests for the underlying useDataManagerBase hook in the common package
  • The hook is part of a broader database interaction system with other similar hooks (useTeam, useCharacter, useBuild)
  • All these hooks follow the same pattern of using useDataManagerBase

Action items:

  • Add unit tests for useDataManagerBase in libs/common/database-ui/src/hooks/ as it's the core functionality
  • Add integration tests for useLightCone to verify database interactions
  • Consider adding a test suite that covers all database hooks consistently
🔗 Analysis chain

Verify test coverage for the hook.

Since this hook handles critical database interactions, ensure it has proper test coverage.

Let's check for existing tests:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for test files related to the light cone hook
# Expected: Find test files that cover the useLightCone hook functionality

echo "Searching for test files..."
fd -e test.ts -e spec.ts useLightCone

echo "Searching for light cone related test content..."
rg -l "useLightCone" -g "*.test.ts" -g "*.spec.ts"

Length of output: 262


Script:

#!/bin/bash
# Let's try a broader search to understand the testing structure and coverage

# First, let's check if there are any test files in the same directory
echo "Checking for test files in the same directory..."
fd -e test.ts -e spec.ts . libs/sr/db-ui/src/hooks/

# Let's also check for any files that might be testing this functionality
# even if not directly named after the hook
echo "Checking for test files containing references to light cones..."
rg -l "lightCone" -g "*.test.ts" -g "*.spec.ts"

# Let's also check for tests of the base hook that this component uses
echo "Checking for tests of the base hook..."
rg -l "useDataManagerBase" -g "*.test.ts" -g "*.spec.ts"

# Let's examine the implementation of useDataManagerBase to understand test coverage needs
echo "Checking useDataManagerBase implementation..."
rg -A 10 "useDataManagerBase" libs/sr/db-ui/src/

Length of output: 2986


Script:

#!/bin/bash
# Let's check the implementation of useDataManagerBase since it's the core functionality
echo "Checking useDataManagerBase implementation in common package..."
rg -A 10 "export function useDataManagerBase" libs/common/database-ui/src/

# Let's also check for tests in the common package where the base functionality lives
echo "Checking for tests of useDataManagerBase in common package..."
fd -e test.ts -e spec.ts . libs/common/database-ui/src/

# Let's also look for any test files that might be testing the database context
echo "Checking for database context tests..."
rg -l "useDatabaseContext" -g "*.test.ts" -g "*.spec.ts"

Length of output: 1485

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useDataManagerBase } from '@genshin-optimizer/common/database-ui'
import { useDatabaseContext } from '../Context'
import { useDatabaseContext } from '../context'
export function useTeam(teamId: string) {
const { database } = useDatabaseContext()
return useDataManagerBase(database.teams, teamId)
Expand Down
3 changes: 3 additions & 0 deletions libs/sr/db-ui/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from './context'
export * from './hooks'
export * from './provider'
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import {
import { SroDatabase } from '@genshin-optimizer/sr/db'
import type { ReactNode } from 'react'
import { useCallback, useMemo, useState } from 'react'
import {
DatabaseContext,
type DatabaseContextObj,
} from '../Context/DatabaseContext'
import { DatabaseContext, type DatabaseContextObj } from '../context'

export function DatabaseProvider({ children }: { children: ReactNode }) {
const dbIndex = parseInt(localStorage.getItem('sro_dbIndex') || '1')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from './DatabaseProvider'
export * from './TeamCalcProvider'
18 changes: 18 additions & 0 deletions libs/sr/db-ui/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"compilerOptions": {
"jsx": "react-jsx",
"allowJs": false,
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true,
"jsxImportSource": "@emotion/react"
},
Comment on lines +2 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding explicit module resolution settings.

While the current configuration works, you might want to consider adding explicit module resolution settings for better clarity:

 {
   "compilerOptions": {
     "jsx": "react-jsx",
     "allowJs": false,
     "esModuleInterop": false,
     "allowSyntheticDefaultImports": true,
     "strict": true,
-    "jsxImportSource": "@emotion/react"
+    "jsxImportSource": "@emotion/react",
+    "moduleResolution": "node",
+    "resolveJsonModule": true
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"compilerOptions": {
"jsx": "react-jsx",
"allowJs": false,
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true,
"jsxImportSource": "@emotion/react"
},
"compilerOptions": {
"jsx": "react-jsx",
"allowJs": false,
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true,
"jsxImportSource": "@emotion/react",
"moduleResolution": "node",
"resolveJsonModule": true
},

"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
}
],
"extends": "../../../tsconfig.base.json"
}
24 changes: 24 additions & 0 deletions libs/sr/db-ui/tsconfig.lib.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../../dist/out-tsc",
"types": [
"node",

"@nx/react/typings/cssmodule.d.ts",
"@nx/react/typings/image.d.ts"
]
Comment on lines +5 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding more essential type definitions.

While the current type definitions cover Node.js and React-specific typings, consider adding other essential types that might be needed:

  • @types/react for React-specific types
  • @types/react-dom for DOM-related React types
 "types": [
   "node",
+  "@types/react",
+  "@types/react-dom",
   "@nx/react/typings/cssmodule.d.ts",
   "@nx/react/typings/image.d.ts"
 ]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"types": [
"node",
"@nx/react/typings/cssmodule.d.ts",
"@nx/react/typings/image.d.ts"
]
"types": [
"node",
"@types/react",
"@types/react-dom",
"@nx/react/typings/cssmodule.d.ts",
"@nx/react/typings/image.d.ts"
]

},
Comment on lines +3 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding important compiler options for type safety and module compatibility.

The current compiler options are minimal. Consider adding these commonly used options for better type safety and module compatibility:

 "compilerOptions": {
   "outDir": "../../../dist/out-tsc",
+  "strict": true,
+  "esModuleInterop": true,
+  "sourceMap": true,
+  "declaration": true,
   "types": [
     "node",
     "@nx/react/typings/cssmodule.d.ts",
     "@nx/react/typings/image.d.ts"
   ]
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"compilerOptions": {
"outDir": "../../../dist/out-tsc",
"types": [
"node",
"@nx/react/typings/cssmodule.d.ts",
"@nx/react/typings/image.d.ts"
]
},
"compilerOptions": {
"outDir": "../../../dist/out-tsc",
"strict": true,
"esModuleInterop": true,
"sourceMap": true,
"declaration": true,
"types": [
"node",
"@nx/react/typings/cssmodule.d.ts",
"@nx/react/typings/image.d.ts"
]
},

"exclude": [
"jest.config.ts",
"src/**/*.spec.ts",
"src/**/*.test.ts",
"src/**/*.spec.tsx",
"src/**/*.test.tsx",
"src/**/*.spec.js",
"src/**/*.test.js",
"src/**/*.spec.jsx",
"src/**/*.test.jsx"
],
"include": ["src/**/*.js", "src/**/*.jsx", "src/**/*.ts", "src/**/*.tsx"]
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider being more specific with file inclusions.

The current include pattern might be too broad. Consider structuring it to match your source code organization:

-"include": ["src/**/*.js", "src/**/*.jsx", "src/**/*.ts", "src/**/*.tsx"]
+"include": [
+  "src/components/**/*",
+  "src/hooks/**/*",
+  "src/utils/**/*",
+  "src/types/**/*"
+]

Committable suggestion was skipped due to low confidence.

}
17 changes: 1 addition & 16 deletions libs/sr/db/src/Database/DataManagers/BuildDataManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,7 @@ export class BuildDataManager extends DataManager<
}
override remove(key: string, notify?: boolean): Build | undefined {
const build = super.remove(key, notify)
// remove data from loadout first
this.database.loadouts.entries.forEach(
([loadoutId, loadout]) =>
loadout.buildIds.includes(key) &&
this.database.loadouts.set(loadoutId, {})
)
// once loadouts are validated, teams can be validated as well
this.database.teams.entries.forEach(
([teamId, team]) =>
team.loadoutMetadata?.some(
(loadoutMetadatum) =>
loadoutMetadatum?.buildId === key ||
loadoutMetadatum?.compareBuildId
) && this.database.teams.set(teamId, {}) // trigger a validation
)

// TODO: remove builds from teams
return build
}
}
16 changes: 1 addition & 15 deletions libs/sr/db/src/Database/DataManagers/BuildTcDataManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,7 @@ export class BuildTcDataManager extends DataManager<
}
override remove(key: string, notify?: boolean): IBuildTc | undefined {
const buildTc = super.remove(key, notify)
// remove data from loadout first
this.database.loadouts.entries.forEach(
([loadoutId, loadout]) =>
loadout.buildTcIds.includes(key) &&
this.database.loadouts.set(loadoutId, {})
)
// once loadouts are validated, teams can be validated as well
this.database.teams.entries.forEach(
([teamId, team]) =>
team.loadoutMetadata?.some(
(loadoutMetadatum) =>
loadoutMetadatum?.buildTcId === key ||
loadoutMetadatum?.compareBuildTcId === key
) && this.database.teams.set(teamId, {}) // trigger a validation
)
// TODO: remove tcbuild from teams?

return buildTc
}
Expand Down
Loading
Loading