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

Bottom navigation for mobile added #150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MHamzaBham
Copy link
Contributor

@MHamzaBham MHamzaBham commented Mar 1, 2025

PR Type

Enhancement


Description

  • Added a bottom navigation bar for mobile view.

  • Introduced hover effects for transaction add button.

  • Integrated a modal for adding/editing transactions.

  • Adjusted z-index for success toast for better visibility.


Changes walkthrough 📝

Relevant files
Enhancement
custom.css
Added hover effects for transaction add button                     

public/assets/css/custom.css

  • Added hover effect for .transaction-add-button.
  • Defined scaling transformations for .transaction-add-button.
  • +11/-1   
    dashboard.blade.php
    Integrated modal for transaction add/edit functionality   

    resources/views/dashboard.blade.php

  • Included a modal for adding/editing transactions.
  • Added a new div with id="modalDiv".
  • +4/-0     
    navigation.blade.php
    Added mobile bottom navigation bar                                             

    resources/views/layouts/navigation.blade.php

  • Added a bottom navigation bar for mobile view.
  • Updated navbar classes for responsive design.
  • Included icons and links in the mobile navigation bar.
  • +58/-26 
    success-toast.blade.php
    Adjusted z-index for success toast                                             

    resources/views/partials/success-toast.blade.php

    • Adjusted z-index for success toast for better visibility.
    +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

    qodo-merge-pro bot commented Mar 1, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Syntax Error

    Extra double quote in onclick attribute of transaction add button which could cause syntax errors

    <a href="#" data-bs-toggle="modal" data-bs-target="#transactionModal" onclick="openModalForAdd()""
    UI Overlap

    Bottom navigation bar might overlap with success toast since both are positioned at the bottom of the screen

    <nav class="d-lg-none fixed-bottom bottom-2 mx-3 px-5 py-2 shadow start-0 end-0 bg-white blur blur-rounded">

    Copy link

    qodo-merge-pro bot commented Mar 1, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix syntax error in onclick
    Suggestion Impact:The commit removed the extra double quote from onclick attribute as suggested

    code diff:

    -                    <a href="#" data-bs-toggle="modal" data-bs-target="#transactionModal" onclick="openModalForAdd()""
    +                    <a href="#" data-bs-toggle="modal" data-bs-target="#transactionModal" onclick="openModalForAdd()"

    The onclick attribute has a syntax error with an extra quotation mark. Remove
    the extra double quote after openModalForAdd() to prevent JavaScript errors.

    resources/views/layouts/navigation.blade.php [126-128]

    -<a href="#" data-bs-toggle="modal" data-bs-target="#transactionModal" onclick="openModalForAdd()""
    +<a href="#" data-bs-toggle="modal" data-bs-target="#transactionModal" onclick="openModalForAdd()"
         class="bg-primary shadow text-white rounded-circle d-flex align-items-center justify-content-center transaction-add-button"
         style="width: 40px; height: 40px;">

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    __

    Why: The extra double quote in the onclick attribute is a syntax error that could prevent the modal from opening properly. This is a critical fix for functionality.

    High
    General
    Improve animation smoothness

    Add transition property to the normal state to ensure smooth animation in both
    directions when hovering.

    public/assets/css/custom.css [38-45]

     .transaction-add-button{
         transform: scale(1);
    +    transition: 0.3s ease-in-out;
     }
     
     .transaction-add-button:hover {
         transform: scale(1.1);
    -    transition: 0.3s ease-in-out;
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 6

    __

    Why: Moving the transition property to the base state ensures smooth animations both when hovering and when returning to normal state, improving the user experience.

    Low
    General
    Fix missing CSS semicolon
    Suggestion Impact:The commit added the missing semicolon after the cursor property value exactly as suggested

    code diff:

     .cursor-pointer{
    -    cursor: pointer
    +    cursor: pointer;

    Add missing semicolon after cursor property value to ensure consistent CSS
    syntax and prevent potential styling issues.

    public/assets/css/custom.css [25-27]

     .cursor-pointer{
    -    cursor: pointer
    +    cursor: pointer;
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 3

    __

    Why: While adding the missing semicolon is technically correct and follows CSS best practices, it's a minor syntax issue that doesn't affect functionality as browsers are forgiving with missing semicolons in CSS.

    Low
    • Update

    @mjawaids
    Copy link
    Collaborator

    mjawaids commented Mar 1, 2025

    @MHamzaBham there are a couple of suggestions from qodo auto code review including syntax error. Could you please look into those?

    Comment on lines 38 to 45
    transform: scale(1);
    }

    .transaction-add-button:hover {
    transform: scale(1.1);
    transition: 0.3s ease-in-out;
    }
    Copy link

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Improve animation smoothness

    Suggested change
    .transaction-add-button{
    transform: scale(1);
    }
    .transaction-add-button:hover {
    transform: scale(1.1);
    transition: 0.3s ease-in-out;
    }
    .transaction-add-button{
    transform: scale(1);
    transition: 0.3s ease-in-out;
    }
    .transaction-add-button:hover {
    transform: scale(1.1);
    }

    @MHamzaBham
    Copy link
    Contributor Author

    @MHamzaBham there are a couple of suggestions from qodo auto code review including syntax error. Could you please look into those?

    Made the suggested changes

    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.

    2 participants