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

Added aria-label tags for twitter image in home page #45

Closed
wants to merge 2 commits into from

Conversation

surya474
Copy link
Contributor

Signed-off-by: surya474 [email protected]

Related to issue

@surya474 surya474 force-pushed the accessibility-alt branch 2 times, most recently from 5ea5157 to 461b4fe Compare October 25, 2019 12:25
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

This looks good generally; I suggested some slightly updated text that is a bit more descriptive.

After those minor updates, I'm happy to approve and merge.

@@ -64,7 +64,7 @@
<hr class="hr has-background-grey">

<div class="buttons">
<a class="button has-background-twitter-blue is-borderless has-text-white" href="{{ $twitterUrl }}">
<a class="button has-background-twitter-blue is-borderless has-text-white" aria-label="Know more in twitter" href="{{ $twitterUrl }}">
Copy link
Member

Choose a reason for hiding this comment

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

"Follow us on Twitter"

@@ -73,7 +73,7 @@
</span>
</a>

<a class="button is-borderless has-background-black has-text-white" href="{{ $githubUrl }}">
<a class="button is-borderless has-background-black has-text-white" aria-label="Know more in Github" href="{{ $githubUrl }}">
Copy link
Member

Choose a reason for hiding this comment

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

"Visit our project on GitHub"

@@ -72,13 +72,13 @@
<div class="navbar-item">
<div class="field is-grouped">
<p class="control">
<a class="button has-background-twitter-blue is-borderless has-text-white" href="{{ $twitterUrl }}">
<a class="button has-background-twitter-blue is-borderless has-text-white" aria-label="Know more in twitter" href="{{ $twitterUrl }}">
Copy link
Member

Choose a reason for hiding this comment

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

"Follow us on Twitter"

<span class="icon">
<i class="fab fa-twitter" aria-hidden="true"></i>
</span>
</a>

<a class="button is-borderless has-background-black has-text-white" href="{{ $githubUrl }}">
<a class="button is-borderless has-background-black has-text-white" aria-label="Know more in Github" href="{{ $githubUrl }}">
Copy link
Member

Choose a reason for hiding this comment

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

"Visit our project on GitHub"

Copy link
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

Needs wording updates requested by Phil

@dmcgowan
Copy link
Member

ping @surya474

please update the wording

@thaJeztah
Copy link
Member

ping @surya474 could you;

  • update the PR, addressing the review comments
  • squash the commits to a single commit
  • update your DCO sign-off to use your real name (currently it's Signed-off-by: surya474 <[email protected]>, please change surya474 to your real name)

@dmcgowan
Copy link
Member

Closing due to inactivity, feel free to re-open the pull request anytime if you continue working on this.

@dmcgowan dmcgowan closed this Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants