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

feat: revised hover effect #1171

Closed

Conversation

dhananjay-Byte
Copy link
Contributor

@dhananjay-Byte dhananjay-Byte commented Nov 6, 2023

closes #1134

👷🏻 Changes made

changed the hover effect of the outline buttons.

📸 Screenshots

Summary by CodeRabbit

  • Documentation

    • Enhanced the README with a hyperlink to the contribution guidelines for pull request titles.
    • Updated Docker setup instructions with a direct link to the Docker Desktop installation guide.
  • Bug Fixes

    • Corrected the API endpoint URL in the userEndpoints object for user reports.
    • Fixed a typo in the HomeBanner component text.
  • Style

    • Removed unused styles related to .mib_numbercircle and adjusted font size in .mib_top_intro > h1.
    • Updated button hover, focus, and active states with new background and text color, and added transition effects.
    • Revised font families and colors in various elements of the Footer component for improved consistency and aesthetics.
  • Chores

    • Removed console logs and unused code from Milaninfobanner and Footer components.
    • Simplified the Modal component by removing unnecessary code related to createPortal.
  • Refactor

    • Changed the Logout function to use a POST request instead of a GET request.

Copy link

coderabbitai bot commented Nov 6, 2023

Walkthrough

The changes across various files in the codebase include updates to documentation, code cleanup, and UI enhancements. A hyperlink was added to the README for better accessibility, Docker setup instructions were made more specific, and an API endpoint was corrected. CSS styles were updated for hover effects and font families, and unnecessary code was removed from components. The Logout function was also modified to use a POST request instead of GET.

Changes

File Path Change Summary
README.md Added hyperlink to "conventional Pull".
docs/DockerSetup.md Updated Docker Desktop installation instructions with a specific link.
src/assets/data/ApiEndpoints.js Updated report endpoint URL.
src/components/Banners/... Removed unused styles and corrected a typo.
src/components/Button/GlobalButton/Button.module.css Updated hover effect styles for buttons.
src/components/Footer/... Updated font families and colors in CSS.
src/components/Footer/Footer.jsx Cosmetic changes to component tags.
src/components/Modal/Modal.jsx Removed createPortal usage and related code.
src/service/MilanApi.js Changed Logout function to use POST request and removed console log.

Assessment against linked issues

Objective Addressed Explanation
#1134: Improve button hover effect to enhance UX The changes in Button.module.css reflect an updated hover effect, aligning with the objective to improve UX.
#1134: Hover effect should indicate navigation The change to the button's hover effect color could indicate navigation, but without context on the UI, it's unclear if it meets the objective.
#1134: Use heading color for hover effect or consider other suggestions The hover effect color was changed, potentially using the heading color as suggested.
#1134: Ensure no duplication of existing issues The assessment cannot be made without access to the project's issue tracker to verify duplication.
#1134: Contributor has read contributing guidelines The assessment cannot be made from the code changes alone; this requires non-codebase evidence.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello @dhananjay-Byte, thank you for raising a pull request.

Currently, the pull request is marked as https://github.com/MilanCommunity/Milan/labels/status%3A%20todo%20%E2%8F%B3 so please wait until the maintainers/owner review it and provide you with feedback/suggestions to proceed further.

Feel free to reach out to Tamal on Twitter, or drop a mail at [email protected] if you think that this pull request is of critical priority.

Give us a ⭐ to show some support
Happy OpenSource 🚀

@github-actions github-actions bot added hacktoberfest-accepted This PR is accepted for Hacktoberfest. status: todo ⏳ This issue is yet to be seen by the maintainer of the project. 💡 feature This generally contains addition/changes to features. labels Nov 6, 2023
Copy link

@Aziz-4980 Aziz-4980 left a comment

Choose a reason for hiding this comment

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

looks good to me

@tamalCodes

This comment was marked as off-topic.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between fbabc50 and d5bfb6d.
Files selected for processing (11)
  • README.md (1 hunks)
  • docs/DockerSetup.md (1 hunks)
  • src/assets/data/ApiEndpoints.js (1 hunks)
  • src/components/Banners/Banner.css (2 hunks)
  • src/components/Banners/HomeBanner.jsx (1 hunks)
  • src/components/Banners/Milaninfobanner.jsx (4 hunks)
  • src/components/Button/GlobalButton/Button.module.css (1 hunks)
  • src/components/Footer/Footer.css (1 hunks)
  • src/components/Footer/Footer.jsx (2 hunks)
  • src/components/Modal/Modal.jsx (2 hunks)
  • src/service/MilanApi.js (1 hunks)
Files skipped from review due to trivial changes (7)
  • README.md
  • docs/DockerSetup.md
  • src/assets/data/ApiEndpoints.js
  • src/components/Banners/HomeBanner.jsx
  • src/components/Banners/Milaninfobanner.jsx
  • src/components/Button/GlobalButton/Button.module.css
  • src/components/Footer/Footer.jsx
Additional comments: 4
src/components/Modal/Modal.jsx (1)
  • 4-8: The removal of createPortal usage suggests a significant change in how the modal is rendered. Ensure that this change does not affect the modal's behavior, especially in terms of accessibility and positioning in the DOM. If the modal was previously being rendered in a specific part of the DOM to ensure proper stacking and accessibility, this change might need to be reconsidered.
src/service/MilanApi.js (1)
  • 103-112: The change from a GET to a POST request for the Logout function is a significant alteration in how the logout functionality is implemented. This should be verified to ensure that it aligns with the backend expectations and that the backend endpoint supports POST requests for logout operations. Additionally, the removal of the console log is a good practice for production code to avoid exposing potentially sensitive information.
src/components/Banners/Banner.css (2)
  • 200-205: The font size for .mib_top_intro > h1 has been reduced from an unspecified larger size to 25px. This change should be verified to ensure it aligns with the intended design and maintains readability across different devices. If this change was made to improve responsiveness or visual hierarchy, it would be beneficial to mention the rationale in the pull request summary or comments for clarity.

  • 273-278: The padding for .mib_mainparent has been reduced from 20.5px to 10.5px on screens with a maximum width of 500px. Additionally, the font size for .mib_top_intro > h1 within the same media query has been reduced from 25px to 23px. These changes should be checked for consistency with the design specifications and to ensure that they do not negatively impact the user experience on smaller screens.

font-size: 0.9rem;
}

#reportForm {
font-family: "Poppins"sans-serif;
Copy link

@coderabbitai coderabbitai bot Nov 21, 2023

Choose a reason for hiding this comment

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

@dhananjay-Byte There is a missing comma between the font family name and the fallback font sans-serif. This could lead to unexpected behavior in how the font is rendered if the primary font is not available.

-  font-family: "Poppins"sans-serif;
+  font-family: "Poppins", sans-serif;

Commitable 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.

Suggested change
font-family: "Poppins"sans-serif;
font-family: "Poppins", sans-serif;

@tamalCodes tamalCodes added status: wip 👩🏻‍💻 This issue/PR is actively being worked on. status: need changes 🚧 Changes have been requested for these PRs and removed hacktoberfest-accepted This PR is accepted for Hacktoberfest. status: todo ⏳ This issue is yet to be seen by the maintainer of the project. labels Nov 21, 2023
@tamalCodes tamalCodes closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 feature This generally contains addition/changes to features. status: need changes 🚧 Changes have been requested for these PRs status: wip 👩🏻‍💻 This issue/PR is actively being worked on.
Projects
Status: Merged/Discarded
Development

Successfully merging this pull request may close these issues.

7 participants