IQBall
/
Application-Web
6
0
Fork 0
Code Issues 49 Pull Requests Packages Projects 5 Releases Wiki Activity
Labels Milestones

Export, Import, Remove, Duplicate and preview tactics #120

Merged
maxime.batista merged 5 commits from home/tactic-management into master 1 year ago
Conversation 0 Commits 5 Files Changed 44
maxime.batista commented 1 year ago

This pull request adds import/export features, and the ability to remove, duplicate and preview a tactic in the home page.

image

This pull request adds import/export features, and the ability to remove, duplicate and preview a tactic in the home page. ![image](/attachments/8ec3d8d7-2d71-41fa-9328-44d88c895a30)
image.png
88 KiB
maxime.batista added 4 commits 1 year ago
continuous-integration/drone/push Build is passing Details
eef1e16830 add JSON export
continuous-integration/drone/push Build is passing Details
a1f8ae5017 add tactic import and duplication in home page
maxime.batista requested review from clement.freville2 1 year ago
maxime.batista requested review from yanis.dahmane-bounoua 1 year ago
maxime.batista requested review from vivien.dufour 1 year ago
maxime.batista requested review from mael.daim 1 year ago
maxime.batista requested review from samuel.berion 1 year ago
maxime.batista force-pushed home/tactic-management from a1f8ae5017 to 47d81bb665 1 year ago
clement.freville2 requested changes 1 year ago
src/domains/TacticContentDomains.ts
return mapIdentifiers(content, (id) => id + "-parent")
}
export function mapIdentifiers(
clement.freville2 commented 1 year ago

This seems to be quite a huge workaround when the primary issue is that the visualizer use document to get the DOM nodes by their id. Maybe update the BendableArrow component to work only in its area prop?

This seems to be quite a huge workaround when the primary issue is that the visualizer use `document` to get the DOM nodes by their id. Maybe update the `BendableArrow` component to work only in its `area` prop?
src/domains/TacticPersistenceDomain.ts
if (typeof contentResult === "string") throw Error(contentResult)
Atomics.add(totalStepsCompleted, 0, 1)
onProgress((Atomics.load(totalStepsCompleted, 0) / treeSize) * 100)
clement.freville2 commented 1 year ago 
-        Atomics.add(totalStepsCompleted, 0, 1)
-        onProgress((Atomics.load(totalStepsCompleted, 0) / treeSize) * 100)
+        totalStepsCompleted += 1
+        onProgress((totalStepsCompleted / treeSize) * 100)

No need to use atomics as everything happen on the same thread.

This work should really be done server side, as it creates a lot of requests and exchanges with the database. Also for the record, you could perform all requests at the same time, regardless of the depth of the node in the tree:

    type NodeInStack = [node: StepInfoNode, parent: number]

    let treeSize = 0
    let completedRequests = 0
    const requests: Promise<StepContent>[] = []
    const nodes: TacticStep[] = []
    const stack: NodeInStack[] = [[context.stepsTree, -1]]
    while (stack.length) {
        const [node, parent] = stack.pop()!
        treeSize += 1
        requests.push(service.getContent(node.id).then((content) => {
            completedRequests += 1
            onProgress((completedRequests / treeSize) * 100)
            if (typeof content === "string") throw Error(content)
            return content
        }));
        for (const child of node.children) {
            stack.push([child, nodes.length])
        }
        const step: TacticStep = {
            content: { components: [] },
            children: [],
        }
        nodes.push(step)
        if (parent >= 0) {
            nodes[parent].children.push(step)
        }
    }

    const contents = await Promise.all(requests)
    for (let i = 0; i < nodes.length; i++) {
        nodes[i].content.components.push(...contents[i].components)
    }
    return {
        name: context.name,
        courtType: context.courtType,
        root: nodes[0],
    }
```diff - Atomics.add(totalStepsCompleted, 0, 1) - onProgress((Atomics.load(totalStepsCompleted, 0) / treeSize) * 100) + totalStepsCompleted += 1 + onProgress((totalStepsCompleted / treeSize) * 100) ``` No need to use atomics as everything happen on the same thread. This work should really be done server side, as it creates a lot of requests and exchanges with the database. Also for the record, you could perform all requests at the same time, regardless of the depth of the node in the tree: ```ts type NodeInStack = [node: StepInfoNode, parent: number] let treeSize = 0 let completedRequests = 0 const requests: Promise<StepContent>[] = [] const nodes: TacticStep[] = [] const stack: NodeInStack[] = [[context.stepsTree, -1]] while (stack.length) { const [node, parent] = stack.pop()! treeSize += 1 requests.push(service.getContent(node.id).then((content) => { completedRequests += 1 onProgress((completedRequests / treeSize) * 100) if (typeof content === "string") throw Error(content) return content })); for (const child of node.children) { stack.push([child, nodes.length]) } const step: TacticStep = { content: { components: [] }, children: [], } nodes.push(step) if (parent >= 0) { nodes[parent].children.push(step) } } const contents = await Promise.all(requests) for (let i = 0; i < nodes.length; i++) { nodes[i].content.components.push(...contents[i].components) } return { name: context.name, courtType: context.courtType, root: nodes[0], } ```
src/pages/HomePage.tsx Outdated
return { ...state!, tactics: [action.tactic, ...state.tactics] }
case HomePageStateActionKind.REMOVE_TACTIC:
return { ...state!, tactics: state.tactics.filter(t => t.id !== action.tacticId) }
clement.freville2 commented 1 year ago 
-            return { ...state!, tactics: action.tactics }
+            return { ...state, tactics: action.tactics }
 
         case HomePageStateActionKind.UPDATE_TEAMS:
-            return { ...state!, teams: action.teams }
+            return { ...state, teams: action.teams }
 
         case HomePageStateActionKind.SET_EXPORTING_TACTIC:
-            return { ...state!, exportingTacticId: action.tacticId }
+            return { ...state, exportingTacticId: action.tacticId }
 
         case HomePageStateActionKind.ADD_TACTIC:
-            return { ...state!, tactics: [action.tactic, ...state.tactics] }
+            return { ...state, tactics: [action.tactic, ...state.tactics] }
 
         case HomePageStateActionKind.REMOVE_TACTIC:
-            return { ...state!, tactics: state.tactics.filter(t => t.id !== action.tacticId) }
+            return { ...state, tactics: state.tactics.filter(t => t.id !== action.tacticId) }
```diff - return { ...state!, tactics: action.tactics } + return { ...state, tactics: action.tactics } case HomePageStateActionKind.UPDATE_TEAMS: - return { ...state!, teams: action.teams } + return { ...state, teams: action.teams } case HomePageStateActionKind.SET_EXPORTING_TACTIC: - return { ...state!, exportingTacticId: action.tacticId } + return { ...state, exportingTacticId: action.tacticId } case HomePageStateActionKind.ADD_TACTIC: - return { ...state!, tactics: [action.tactic, ...state.tactics] } + return { ...state, tactics: [action.tactic, ...state.tactics] } case HomePageStateActionKind.REMOVE_TACTIC: - return { ...state!, tactics: state.tactics.filter(t => t.id !== action.tacticId) } + return { ...state, tactics: state.tactics.filter(t => t.id !== action.tacticId) } ```
maxime.batista marked this conversation as resolved
src/pages/HomePage.tsx
handleChange={handleDrop}
types={["json"]}
hoverTitle="Déposez ici"
label="Séléctionnez ou déposez un fichier ici"></FileUploader>
clement.freville2 commented 1 year ago 
-                label="Séléctionnez ou déposez un fichier ici"></FileUploader>
+                label="Sélectionnez ou déposez un fichier ici" />
```diff - label="Séléctionnez ou déposez un fichier ici"></FileUploader> + label="Sélectionnez ou déposez un fichier ici" /> ```
maxime.batista marked this conversation as resolved
src/pages/popup/ExportTacticPopup.tsx
export interface ExportTacticPopupProps {
service: TacticService
show: boolean
clement.freville2 commented 1 year ago 
-    show: boolean

If the popup has been rendered, it is because it should be shown.

```diff - show: boolean ``` If the popup has been rendered, it is because it should be shown.
maxime.batista marked this conversation as resolved
src/pages/popup/ExportTacticPopup.tsx
window.addEventListener("keyup", onKeyUp)
return () => window.removeEventListener("keyup", onKeyUp)
}, [onHide])
clement.freville2 commented 1 year ago

Place this code in a separate hook as it is used for all popups.

Place this code in a separate hook as it is used for all popups.
maxime.batista marked this conversation as resolved
src/pages/popup/ExportTacticPopup.tsx
)
setExportPercentage(0)
}}>
<p className="export-card-title">Export in JSON</p>
clement.freville2 commented 1 year ago 
-                        <p className="export-card-title">Export in JSON</p>
+                        <p className="export-card-title">Exporter en JSON</p>
```diff - <p className="export-card-title">Export in JSON</p> + <p className="export-card-title">Exporter en JSON</p> ```
maxime.batista marked this conversation as resolved
src/pages/popup/ExportTacticPopup.tsx
const e = document.createElement("a")
e.setAttribute(
"href",
"data:application/octet-stream," +
clement.freville2 commented 1 year ago 
-        "data:application/octet-stream," +
+        "data:application/json;charset=utf-8," +
```diff - "data:application/octet-stream," + + "data:application/json;charset=utf-8," + ```
maxime.batista marked this conversation as resolved
src/style/home/home.css
align-items: center;
justify-content: center;
* {
clement.freville2 commented 1 year ago

While it's great to try new things, note that native CSS selector nesting has only been supported for less than a year (it's not available in the current Firefox ESR, for example).

While it's great to try new things, note that native [CSS selector nesting](https://caniuse.com/css-nesting) has only been supported for less than a year (it's not available in the current Firefox ESR, for example).
maxime.batista marked this conversation as resolved
src/style/home/personnal_space.css
}
#body-personal-space::-webkit-scrollbar {
display: none;
clement.freville2 commented 1 year ago 
-#body-personal-space::-webkit-scrollbar {
-    display: none;
+#body-personal-space {
+    overflow-y: hidden;
```diff -#body-personal-space::-webkit-scrollbar { - display: none; +#body-personal-space { + overflow-y: hidden; ```
maxime.batista marked this conversation as resolved
src/style/template/header.css
width: 50px;
height: 50px;
border-radius: 20%;
-webkit-user-drag: none;
clement.freville2 commented 1 year ago 
-    -webkit-user-drag: none;
```diff - -webkit-user-drag: none; ```
maxime.batista marked this conversation as resolved
maxime.batista added 1 commit 1 year ago
continuous-integration/drone/push Build is passing Details
262cf97445 Apply suggestions
maxime.batista merged commit 0de42db300 into master 1 year ago
maxime.batista deleted branch home/tactic-management 1 year ago

Reviewers

samuel.berion was requested for review 1 year ago
vivien.dufour was requested for review 1 year ago
mael.daim was requested for review 1 year ago
yanis.dahmane-bounoua was requested for review 1 year ago
clement.freville2 requested changes 1 year ago
continuous-integration/drone/push Build is passing
Details
The pull request has been merged as 0de42db300.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
samuel.berion
vivien.dufour
mael.daim
yanis.dahmane-bounoua
clement.freville2
Labels
Apply labels
Clear labels
bug
Something is not working duplicate
This issue or pull request already exists enhancement
New feature help wanted
Need some help invalid
Something is wrong question
More information is needed wontfix
This won't be fixed
No Label bug duplicate enhancement help wanted invalid question wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: IQBall/Application-Web#120
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'home/tactic-management'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes