Andras/BoundlessFlowCampus2K

Fork 0

issue-836-add-card-placeholders #848

Merged

a22erigr merged 6 commits from issue-836-add-card-placeholders into team_2_week_5

2026-05-13 08:37:06 +00:00

b25marem commented

2026-05-08 10:59:19 +00:00

Collaborator

Implemented issue #836 by adding interactive placeholders for empty dashboard grid cells in edit mode.

Changes:

Added “+ Add card” placeholders for empty grid positions
Placeholders only appear in edit mode
Clicking a placeholder opens Add Card modal
New cards are now added at the selected grid position instead of the first available spot
Placeholder items are filtered out from saved layout data

Implemented issue #836 by adding interactive placeholders for empty dashboard grid cells in edit mode. Changes: * Added “+ Add card” placeholders for empty grid positions * Placeholders only appear in edit mode * Clicking a placeholder opens Add Card modal * New cards are now added at the selected grid position instead of the first available spot * Placeholder items are filtered out from saved layout data

b25marem added the

needs review

label

2026-05-08 10:59:19 +00:00

b25marem added 2 commits

2026-05-08 10:59:19 +00:00

Add edit mode placeholders for empty dashboard grid cells 9f4276e9a5

Use selected grid position when adding dashboard cards 957034703d

a22erigr

2026-05-08 14:51:41 +00:00

added the
Under-review
label
requested review from c24elipe

c24elipe commented

2026-05-11 13:29:45 +00:00

Collaborator

Review on #848

What's done

Tested on Arch Linux using Firefox

App starts
- It compiles without errors.
- Cards can be added without a problem.
- Cards can scale down... but not up...

Inspected the following files

frontend/src/pages/dashboard/Dashboard.js:
- Indentation/code styling
  - Follows Prettier.
- Naming conventions
  - The rule "All variable names should be written in camelCase and not shortened" is not followed in:
    Lines like const { col } = gridSizes[...], and onChange={(v) => updateCardValue(item.i, v)}. I think const cell = { x, y, w: 1, h: 1 }; might be okay since "x, y, w: 1, h: 1" are values that are used by the underlying library. I'm not sure if they are still in violation, though.
- Comments
  - They exist, and I would say they are mostly good. However, functions like "findFirstAvailableSpot" and values like isColliding and viewRegister could use some comments to explain what they are and/or do.
  - On lines 299–300, there is a multiline comment. It should use /* */ instead of two // for comments.
- General code quality
  - Since AddNewView isn't a component view but a logic function, it should be refactored to addNewView.
frontend/src/pages/dashboard/utils/AddNewView.js
- Indentation/code styling
  - Follows Prettier.
- Naming conventions
  - Since AddNewView isn't a component view but a logic function, it should be refactored to addNewView.
  - Again, shorthands for variables shouldn't be used. const w = sizeConfig.minW;, and const h = sizeConfig.minH;. They aren't acceptable here since the function doesn't interact with the library directly.
- Comments
  - The multiline comment syntax (/* */) should be used on lines 21–22, since it is a multiline comment.
  - Adding more comments on lines 60–61 would be great, as I couldn't tell what they were meant to do at first glance.
- General code quality
  - The if-block could potentially be changed to a switch-block. This is likely a non-issue and just a personal preference of the reviewer.

What needs to be done

BUGS:

A major bug was found: the cards can be placed and shrunk, but not expanded. This needs to be fixed!

TO DO:

Dashboard.js:
- Update the names of the variables to not use shorthands.
- Add some more comments to functions like findFirstAvailableSpot and values like isColliding and viewRegister.
- Update lines 299–300 to use a multiline comment.
- Update the name of AddNewView to addNewView.
AddNewView.js
- Update the name of AddNewView to addNewView.
- Update lines 21–22 to use a multiline comment.
- In general, add a few more comments, specifically for sections like lines 60–61.

Conclusion

Priority 1 should be to add the ability to grow a card's size.
Then fix the merge conflict.
After both of those, address the to-do list above.
For the moment, this PR shouldn't be merged.

# Review on #848 ## What's done ### Tested on Arch Linux using Firefox + App starts + It compiles without errors. + Cards can be added without a problem. + Cards can scale down... but not up... ### Inspected the following files * **frontend/src/pages/dashboard/Dashboard.js**: * **Indentation/code styling** * Follows Prettier. * **Naming conventions** * The rule "All variable names should be written in camelCase and not shortened" is not followed in: Lines like _const { col } = gridSizes[...]_, and _onChange={(v) => updateCardValue(item.i, v)}_. I think _const cell = { x, y, w: 1, h: 1 };_ might be okay since "x, y, w: 1, h: 1" are values that are used by the underlying library. I'm not sure if they are still in violation, though. * **Comments** * They exist, and I would say they are mostly good. However, functions like "findFirstAvailableSpot" and values like isColliding and viewRegister could use some comments to explain what they are and/or do. * On lines 299–300, there is a multiline comment. It should use **/\* \*/** instead of two **//** for comments. * **General code quality** * Since _AddNewView_ isn't a component view but a logic function, it should be refactored to **addNewView**. * **frontend/src/pages/dashboard/utils/AddNewView.js** * **Indentation/code styling** * Follows Prettier. * **Naming conventions** * Since _AddNewView_ isn't a component view but a logic function, it should be refactored to **addNewView**. * Again, shorthands for variables shouldn't be used. _const w = sizeConfig.minW;_, and _const h = sizeConfig.minH;_. They aren't acceptable here since the function doesn't interact with the library directly. * **Comments** * The multiline comment syntax (**/\* \*/**) should be used on lines 21–22, since it is a multiline comment. * Adding more comments on lines 60–61 would be great, as I couldn't tell what they were meant to do at first glance. * **General code quality** * The if-block could potentially be changed to a switch-block. This is likely a non-issue and just a personal preference of the reviewer. ## What needs to be done **BUGS**: * A major bug was found: the cards can be placed and shrunk, but not expanded. This needs to be fixed! TO DO: * **Dashboard.js**: * Update the names of the variables to not use shorthands. * Add some more comments to functions like **findFirstAvailableSpot** and values like **isColliding** and **viewRegister**. * Update lines 299–300 to use a multiline comment. * Update the name of **AddNewView** to **addNewView**. * **AddNewView.js** * Update the name of **AddNewView** to **addNewView**. * Update lines 21–22 to use a multiline comment. * In general, add a few more comments, specifically for sections like lines 60–61. ## Conclusion Priority 1 should be to add the ability to grow a card's size. Then fix the merge conflict. After both of those, address the to-do list above. For the moment, this PR shouldn't be merged.

c24elipe added

reviewed

and removed

needs review

Under-review

labels

2026-05-11 13:29:51 +00:00

a22erigr commented

2026-05-11 18:10:08 +00:00

Collaborator

@b25marem Can you fix the issues?

👍 1

a22erigr added the

further-changes-needed

label

2026-05-11 18:13:39 +00:00

b25marem added 1 commit

2026-05-12 12:15:10 +00:00

Fix dashboard card resize and review comments cf77db400d

b25marem commented

2026-05-12 12:16:22 +00:00

Author

Collaborator

@c24elipe Thanks for the review. I have now fixed the card resize issue and addressed the review comments regarding naming, comments, and the AddNewView refactor. You can take another look when you have time.

b25marem added 1 commit

2026-05-12 12:29:23 +00:00

Merge team_2_week_5 into issue-836-add-card-placeholders edd4a31f26

c24elipe removed the

reviewed

label

2026-05-12 13:29:42 +00:00

b25marem added 1 commit

2026-05-12 14:20:08 +00:00

Fix add card placeholder interaction bug cda518017e

b25marem added 1 commit

2026-05-12 14:27:51 +00:00

Improve add card placeholder behavior and comments cb92e04aff

a22erigr requested review from a24nedka

2026-05-13 07:49:11 +00:00

a24nedka commented

2026-05-13 08:36:16 +00:00

Collaborator

Reviewed the improvement.

The card can now be expanded.
Comments , naming and AddNew refactor is good.
Overall good improvement.

Conclusion:
The solution is ready to be merged .

**Reviewed the improvement.** - The card can now be expanded. - Comments , naming and AddNew refactor is good. - Overall good improvement. **Conclusion:** The solution is ready to be merged .

a22erigr merged commit a79215f4a8 into team_2_week_5

2026-05-13 08:37:06 +00:00

a22erigr referenced this pull request from a commit

2026-05-13 08:37:07 +00:00

Merge pull request 'issue-836-add-card-placeholders' (#848) from issue-836-add-card-placeholders into team_2_week_5

b25marem referenced this pull request from a commit

2026-05-15 08:56:06 +00:00

Merge pull request 'issue-836-add-card-placeholders' (#848) from issue-836-add-card-placeholders into team_2_week_5