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

fix(web): Add margin for different screen size #820

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

wp043
Copy link

@wp043 wp043 commented Mar 10, 2025

User description

Description

This pull request includes adjusting margins for the headline "Secure Your Configurations with Confidence" in the web under the component in web/src/components/secretSection/index.tsx file. The change aims to improve visibility of information across different devices, especially those with 1024px width.

Fixes #818

Future Improvements

Current changes primarily focus on laptop view as most target users may access the website through laptop. Some uncommon dimensions of devices may need further consideration.

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Bug fix


Description

  • Adjusted section margins for better headline visibility.

  • Improved responsiveness for various screen sizes and orientations.

  • Ensured headline is fully visible across devices.


Changes walkthrough 📝

Relevant files
Bug fix
index.tsx
Adjust section styling for headline visibility                     

apps/web/src/components/secretSection/index.tsx

  • Adjusted section margins for better responsiveness.
  • Added padding and minimum height for improved layout.
  • Modified margin-top for landscape and small screen views.
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    818 - PR Code Verified

    Compliant requirements:

    • Fix the UI bug where part of the headline "Secure Your Configurations with Confidence" is hidden under a div

    Requires further human verification:

    • Ensure the headline is clearly visible on the website (https://keyshade.xyz/) - This requires visual verification across different screen sizes

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Screen Size Compatibility

    The landscape mode has a large top margin (30vh) which might push content too far down on some devices. Consider testing this value on various screen orientations.

    <section className="flex min-h-[50vh] w-full flex-col items-center gap-y-[5rem] p-6 sm:mt-[1vh] md:gap-y-[9.69rem] landscape:mt-[30vh]">

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Reduce excessive margin

    The margin-top value for landscape mode (30vh) is excessive and could push
    content completely off-screen on landscape-oriented devices. Consider using a
    more moderate value like 5vh-10vh for landscape orientation.

    apps/web/src/components/secretSection/index.tsx [53]

    -<section className="flex min-h-[50vh] w-full flex-col items-center gap-y-[5rem] p-6 sm:mt-[1vh] md:gap-y-[9.69rem] landscape:mt-[30vh]">
    +<section className="flex min-h-[50vh] w-full flex-col items-center gap-y-[5rem] p-6 sm:mt-[1vh] md:gap-y-[9.69rem] landscape:mt-[8vh]">
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that a 30vh top margin in landscape mode is excessive and could push content off-screen. Reducing it to 8vh is a reasonable improvement that would enhance the user experience on landscape-oriented devices.

    Medium
    • More

    @rajdip-b
    Copy link
    Member

    @wp043 hey! could you please drop screenshots of the fix in action?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    WEB: Headline "Secure Your Configurations with Confidence" message is hidden under a div
    2 participants