Contribute element container exceeds screen width on mobile #919 #959
Conversation
There was a problem hiding this comment.
Please fix CI/CD checks
https://github.com/OWASP/Nest/blob/main/CONTRIBUTING.md#contributing-workflow
WalkthroughThe changes update the structure and behavior of two files. In the HTML file, the doctype and viewport meta tag have been corrected and enhanced. New CSS styles and content sections have been added, along with a script that logs a message on DOM content load while removing an older script reference. In the MultiSearch component, event handling for keyboard interactions has been fortified with optional chaining and conditional checks, along with minor formatting improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant B as Browser
participant D as Document
participant S as Script
participant C as Console
B->>D: Load HTML Page
D->>S: Trigger DOMContentLoaded event
S->>C: Log "Page loaded successfully"
sequenceDiagram
participant U as User
participant MS as MultiSearch Component
participant KH as Key Handler
participant HC as handleSuggestionClick
U->>MS: Presses keyboard key
MS->>KH: Processes key event with index checks
alt Suggestion exists
KH->>HC: Invoke handleSuggestionClick
else No valid suggestion
KH-->>U: No action taken
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/index.html(1 hunks)frontend/src/components/MultiSearch.tsx(1 hunks)
🔇 Additional comments (5)
frontend/src/components/MultiSearch.tsx (1)
90-97: Improved error handling with optional chainingThe changes enhance safety by adding optional chaining (
?.) when accessing the suggestions array and verifying that a suggestion exists before callinghandleSuggestionClick. This prevents potential runtime errors when dealing with invalid indices.frontend/index.html (4)
1-5: Fix DOCTYPE declaration and review viewport restrictionsThe DOCTYPE declaration is now correctly capitalized. However, the viewport meta tag includes
user-scalable=nowhich prevents users from zooming on mobile devices.The
user-scalable=noparameter can cause accessibility issues for users with visual impairments who need to zoom content. Consider removing this restriction unless there's a specific reason that requires preventing zoom.
8-61: Good responsive design implementationThe added CSS provides a well-structured responsive layout with:
- Proper box-sizing reset
- Flexbox centering
- Responsive container with max-width constraints
- Media queries for smaller screens
This directly addresses the PR objective of fixing the "container exceeds screen width on mobile" issue by setting
max-width: 100%and proper overflow handling.
63-76: New container structure aligns with PR objectivesThe implementation of the new container structure with separate sections for Projects and Contribute resolves the width issue mentioned in the PR objectives. The container and sections now have:
- Proper width constraints (
max-width: 100%)- Consistent padding
- Proper overflow handling
This change successfully addresses the reported issue of content exceeding screen width on mobile devices.
77-81: Simple DOM ready event handlerThe script adds a basic DOM content loaded event handler that logs a message to the console.
This appears to be a placeholder. Consider whether this should be expanded to include initialization code or if it's intended to be replaced by the React application entry point script in the future.
| const { index, subIndex } = highlighted ; | ||
| if (subIndex < suggestions[index].hits.length - 1) { |
There was a problem hiding this comment.
Fix variable name typo
There's a reference to a non-existent variable highlighted which would cause a runtime error. This should be highlightedIndex as used elsewhere in the component.
- const { index, subIndex } = highlighted ;
+ const { index, subIndex } = highlightedIndex;Also, please remove the extra spaces before the semicolon.
📝 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.
| const { index, subIndex } = highlighted ; | |
| if (subIndex < suggestions[index].hits.length - 1) { | |
| const { index, subIndex } = highlightedIndex; | |
| if (subIndex < suggestions[index].hits.length - 1) { |
|
| const { index, subIndex } = highlightedIndex | ||
| if (subIndex < suggestions[index].hits.length - 1) { | ||
| const { index, subIndex } = highlightedIndex ; | ||
| if (subIndex < suggestions[index].hits.length - 1) { |
There was a problem hiding this comment.
Can we clean this up? The spaces before ; and the extra space before if statement?
You should run an auto-formatter that would fix these for you.
There was a problem hiding this comment.
Also, is it guaranteed that suggestions[index].hits is available? Do we need to add ? maybe?
| <div id="root"></div> | ||
| <script type="module" src="/src/main.tsx"></script> | ||
| </body> | ||
| <style> |
There was a problem hiding this comment.
Why do we have style here in the middle of the template of all the places? 🤔
and we normally do not have CSS in our files
There was a problem hiding this comment.
I'm not sure I understand what's going on here at all.. 👀
This is how the page looks now:
|
You also need to address CI/CD failing before next round of review |
|
ok ,i will try to fix these issues before next round of review |
Resolves #919
Add the PR description here.
The changes made focused on improving mobile responsiveness by fixing the Contribute section’s width issue. The meta viewport tag was adjusted to ensure proper scaling and prevent zooming inconsistencies. A container was introduced to wrap both the Projects and Contribute sections, ensuring they share the same width. The CSS was modified to set max-width: 100% for both sections, preventing overflow beyond the screen. A flexbox-based layout was implemented to center the content and maintain a structured UI. Additionally, responsive styles were added to adjust padding and spacing on smaller screens.
Summary by CodeRabbit