Andras/BoundlessFlowCampus2K
3
6
Fork 0
Code Issues 143 Pull requests 14 Projects Releases Packages Wiki Activity Actions 2

#668-Add-preview-header #696

Merged
a22erigr merged 8 commits from #668-Add-preview-header into team_2_week_4 2026-05-06 06:34:31 +00:00
Conversation 7 Commits 8 Files changed 2 +28 -9
a24danwe commented 2026-05-04 10:30:07 +00:00
Collaborator
Copy link

Adds "Preview" over the selected view.

Adds "Preview" over the selected view.
b25marem commented 2026-05-05 07:23:51 +00:00
Collaborator
Copy link

Review on #668

Whats done

Tested on Windos using Chrome

  • Verified that "Preview" is displayed above selected view
  • Tested selecting different views and confirmed preview updates correctly.
  • App starts
    +No crashes when opening Add Card modal
    • Tested multiple views and preview randering
  • Layout appears centered and spaced

Inspected following files

  • frontend/src/features/addCardModal/AddCardModal.js

    • indentation is ok

    • Naming convention

      • Follows React and camelCase conventions
      • Comment
        +Acceptable level of comments
      • General code quality
        +Code is readable and structured clearly
      • frontend/src/features/addCardModal/AddCardModal.module.css
      • Indentation/code styling
      • Follows CSS formatting standard
        +Naming convention
        • Does not follow convention
        • Uses .preview-header instead of matching JSX usage
      • Comments
        +Missing comment for new rule set
      • General code quality
        +Styling is simple and understandable

    • [What needs to be done

      • AddCardModal.module.css + AddCardModal.js
        • Fix mismatch between CSS module class and JSX usage
        • JSX uses:
          +styles.previewHeader
          +CSS defines:
          • .preview-header
            +These must match for CSS modules to apply styling correctly
            +Rename CSS class to .previewHeader or update JSX to styles[preview-header]
            +AddCardModal.module.css
            +Add descriptive comment above the preview header rule set

Conclusion

+Feature works as intended in the UI
+However, styling works due to existing layout, not the new CSS class

  • Not ready for merge until naming issue is fixed
# Review on #668 ## Whats done ### Tested on Windos using Chrome + Verified that "Preview" is displayed above selected view + Tested selecting different views and confirmed preview updates correctly. + App starts +No crashes when opening Add Card modal + Tested multiple views and preview randering + Layout appears centered and spaced ### Inspected following files + frontend/src/features/addCardModal/AddCardModal.js + indentation is ok + Naming convention + Follows React and camelCase conventions + Comment +Acceptable level of comments + General code quality +Code is readable and structured clearly + frontend/src/features/addCardModal/AddCardModal.module.css + Indentation/code styling + Follows CSS formatting standard +Naming convention + Does not follow convention + Uses .preview-header instead of matching JSX usage + Comments +Missing comment for new rule set + General code quality +Styling is simple and understandable + [What needs to be done + AddCardModal.module.css + AddCardModal.js + Fix mismatch between CSS module class and JSX usage + JSX uses: +styles.previewHeader +CSS defines: + .preview-header +These must match for CSS modules to apply styling correctly +Rename CSS class to .previewHeader or update JSX to styles[preview-header] +AddCardModal.module.css +Add descriptive comment above the preview header rule set ## Conclusion +Feature works as intended in the UI +However, styling works due to existing layout, not the new CSS class + Not ready for merge until naming issue is fixed
b25marem commented 2026-05-05 07:25:31 +00:00
Collaborator
Copy link

image

![image](/attachments/3be36e8f-7fee-4e80-a32a-aa4a6d7368dc)
image.png
13 KiB
a24danwe commented 2026-05-05 08:41:41 +00:00
Author
Collaborator
Copy link

Thank you for the review!

I have updated AddCardModal.js to match the naming convention and I have also added comments to AddCardModal.module.css.

Thank you for the review! I have updated AddCardModal.js to match the naming convention and I have also added comments to AddCardModal.module.css.
👍 1
c24elipe reviewed 2026-05-05 21:14:12 +00:00
c24elipe left a comment
Collaborator
Copy link

On a quick overview, the code looks good.
Will now look over it properly in the editor.

On a quick overview, the code looks good. Will now look over it properly in the editor.
1. Fixed a spelling mistake  categoires -> categories
2. Removed a none existent style class.
3. Removed unnecessary variable
4. Ran prettier to double check
c24elipe commented 2026-05-05 22:07:16 +00:00
Collaborator
Copy link

Re-Review on #668

Whats done

Tested on cachyOs (Arch Linux) and Brave browser.

  • The first steps where similar to what was done in the first review. Mainly compiling and opening the add card menu to seeing that the text shows up. It did.

image

Inspected the following files

  • frontend/src/features/addCardModal/AddCardModal.js:

    • Indentation:
      • The prettier config is used and therefore good.
    • Comment:
      • As someone with lacking react-knowledge, I would say it is a suitable amount of comments.
    • Naming convention:
      • The convention, as far as I can tell, is being followed well.
    • General code quality:
      • Some code problems were found. However, those where outside the scope of this PR and unrelated to the code created. Therefore, I saw it fit to just fix the 3 minor code problems found. The results of which can be seen in the commit above. No problem with the code otherwise.

  • frontend/src/features/addCardModal/AddCardModal.module.css:

    • Indentation:
      • Again, the prettier config is used and therefore good.
    • Comment:
      • The file overall goes against the Coding Conventions for CSS. Specifically the rule
        "Have descriptive comments for every rule set". But the problem with that statement is that the parts that go against the conventions aren't added in this PR and already exist. So, it is a question if that is something to fix in this PR or later?.
    • Naming convention:
      • Like before, the code that was added in this PR follows the naming standard. But the older code doesn't. Specifically, the rules that all names should be lower case and use a hyphen between words.
    • General code quality:
      • Just because I love repeating myself. The code that was added in this PR is correctly placed and holds the standard for the code we aim to keep. But again, the older code is placed in a non-alphabetical order, lacks comments, and doesn't follow the naming standard.

What needs to be done

Nothing need to be done with the code created for this Issue and PR.
But since there are several issues with, let's be honest, most code conventions in the overall "AddCardModal.module.css" file, it is worth contemplating if it should be fixed before merging.

Conclusion

The code can be merged if the issues in "AddCardModal.module.css" can be ignored .

# Re-Review on #668 ## Whats done ### Tested on cachyOs (Arch Linux) and Brave browser. * The first steps where similar to what was done in [the first review](https://git.webug.se/Andras/BoundlessFlowCampus2K/pulls/696#issuecomment-7469). Mainly compiling and opening the add card menu to seeing that the text shows up. It did. ![image](/attachments/aa72dcf6-8caa-4947-9bde-cfd2d61d2fd7) ### Inspected the following files * **frontend/src/features/addCardModal/AddCardModal.js**: * **Indentation**: * The prettier config is used and therefore good. * **Comment**: * As someone with lacking react-knowledge, I would say it is a suitable amount of comments. * **Naming convention**: * The convention, as far as I can tell, is being followed well. * **General code quality**: * Some code problems were found. However, those where outside the scope of this PR and unrelated to the code created. Therefore, I saw it fit to just fix the 3 minor code problems found. The results of which can be seen in the commit above. No problem with the code otherwise. * **frontend/src/features/addCardModal/AddCardModal.module.css**: * **Indentation**: * Again, the prettier config is used and therefore good. * **Comment**: * The file overall goes against the Coding Conventions for CSS. Specifically the rule "Have descriptive comments for every rule set". But the problem with that statement is that the parts that go against the conventions aren't added in this PR and already exist. So, it is a question if that is something to fix in this PR or later?. * **Naming convention**: * Like before, the code that was added in this PR follows the naming standard. But the older code doesn't. Specifically, the rules that all names should be lower case and use a hyphen between words. * **General code quality**: * Just because I love repeating myself. The code that was added in this PR is correctly placed and holds the standard for the code we aim to keep. But again, the older code is placed in a non-alphabetical order, lacks comments, and doesn't follow the naming standard. ### What needs to be done Nothing need to be done with the code created for this Issue and PR. But since there are several issues with, let's be honest, most code conventions in the overall "_AddCardModal.module.css_" file, it is worth contemplating if it should be fixed before merging. ## Conclusion The code can be merged if the issues in "_AddCardModal.module.css_" can be ignored .
image.png
172 KiB
c24elipe approved these changes 2026-05-05 22:07:34 +00:00
a22erigr commented 2026-05-06 06:34:24 +00:00
Collaborator
Copy link

Nothing need to be done with the code created for this Issue and PR.
But since there are several issues with, let's be honest, most code conventions in the overall "AddCardModal.module.css" file, it is worth contemplating if it should be fixed before merging.

Unfortunately we don't have time to fix the overall code convention in AddCardModal.module.css before weekly merge. I will create a issue on this.

_Nothing need to be done with the code created for this Issue and PR. But since there are several issues with, let's be honest, most code conventions in the overall "AddCardModal.module.css" file, it is worth contemplating if it should be fixed before merging._ Unfortunately we don't have time to fix the overall code convention in AddCardModal.module.css before weekly merge. I will create a issue on this.
a22erigr merged commit 33150792a8 into team_2_week_4 2026-05-06 06:34:31 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
b25marem
c24elipe
Labels
Clear labels   
_CRITICAL_

Only use this label for critical tasks that are project-scope,and must be addressed ASAP.

  
API

   
app

   
backEnd

   
Blocked-waiting-for-further-changes

   
bug

Something is not working

  
bug-only-on-server-for-mobile-not-webpage

   
Bug-Report-After-Merge

   
cleanup

Issues to cleanup the codebase (restructure folders, remove deprecated or smelly code)

  
close

Issue to close

  
design

   
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
feature request

   
frontEnd

   
further-changes-needed

   
future-problem-not-fixint-this-period

   
help wanted

Need some help

  
invalid

Something is wrong

  
last-week-issue-to-fix

   
library

   
low-priority

   
needs input

Issue needs input from product owner or customer

  
needs review

issue is in need of review

  
not-implemented.

   
project documentation

   
question

More information is needed

  
research

   
reviewed

This Issue has been reviewed

  
Script

   
security

use this for security-related issues

  
SQL

  
style

   
Team 1

Team 1 is responsible for this issue

  
Team 2

Team 2 is responsible for this issue

  
team leaders

   
test-creation

   
testing

   
topLevel

Reserved for top-level, project-wide issues.

  
unassigned

   
Under-review

   
wontfix

This won't be fixed

No labels
_CRITICAL_
API
app
backEnd
Blocked-waiting-for-further-changes
bug
bug-only-on-server-for-mobile-not-webpage
Bug-Report-After-Merge
cleanup
close
design
duplicate
enhancement
feature request
frontEnd
further-changes-needed
future-problem-not-fixint-this-period
help wanted
invalid
last-week-issue-to-fix
library
low-priority
needs input
needs review
not-implemented.
project documentation
question
research
reviewed
Script
security
SQL
style
Team 1
Team 2
team leaders
test-creation
testing
topLevel
unassigned
Under-review
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Andras/BoundlessFlowCampus2K!696
No description provided.