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

View Our Menu Button Working #9

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

Conversation

Jyoth1313
Copy link

Done now its working you didnt given Href link

@Jyoth1313 Jyoth1313 changed the title Master View Our Menu Button Working Mar 18, 2024
Copy link

@Imran-imtiaz48 Imran-imtiaz48 left a comment

Choose a reason for hiding this comment

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

The changes focus on maintaining consistency within the "View Our Menu" button. The button includes two spans for text: text-1 (visible) and text-2 (hidden but accessible). This structure ensures both accessibility and smooth animations for the user experience.
Pros:

  1. Semantic HTML: The use of aria-hidden="true" is a good accessibility practice, ensuring that the second span is hidden from screen readers while still serving its visual function.
  2. Consistent Styling: The double span approach is clear, and using the same text for both ensures that the design is aligned.
  3. Good Structure: The button itself is well-structured with a primary btn btn-primary slider-reveal class, ensuring consistent behavior across the site.
    Suggestions for Improvement:
    • DRY Principle: If the exact same button structure is repeated multiple times, consider creating a reusable component or partial (depending on your framework) to avoid repetition and make future changes easier to manage.
    • ARIA-Labelling: To enhance accessibility further, you could use an aria-label on the button itself, which can be a cleaner way of providing accessibility than relying on a hidden span.
    Overall, this is well-done and functional for the purpose, especially regarding accessibility and design.

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

Successfully merging this pull request may close these issues.

2 participants