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.

Sign up for GitHub

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

Jump to bottom

[UPDATE] added story name to aria label #2158

Merged
merged 1 commit into from
Oct 16, 2022
Merged

[UPDATE] added story name to aria label #2158

merged 1 commit into from
Oct 16, 2022

Conversation

nikhilbhatt
Copy link
Contributor

Description

Updated aria label value in story actions with story action & story name

More Details

Corresponding Issue

#2121

Reviewing this pull request? Check out our Code Review Practices guide if you haven't already!

julianguyen
julianguyen requested changes Oct 8, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Thanks so much for taking this on! Looking good, just some feedback and we should be good to go!

client/app/components/Story/StoryActions.jsx Outdated Show resolved Hide resolved
julianguyen
julianguyen requested changes Oct 9, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Also yay for adding a contributor blurb 🎉

There's one more change that needs to be done before we're good to merge!

client/app/components/Story/StoryActions.jsx Outdated Show resolved Hide resolved
doc/pages/blurbs.json Outdated Show resolved Hide resolved
@nikhilbhatt nikhilbhatt force-pushed the improve_aria_labels branch 2 times, most recently from c5903e8 to 0052f48 Compare October 9, 2022 16:50
julianguyen
julianguyen requested changes Oct 9, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Looks like there's some ESLint failures! I recommend running yarn lint:eslint locally so you can fix them.

@nikhilbhatt nikhilbhatt marked this pull request as draft October 9, 2022 17:41
@nikhilbhatt nikhilbhatt force-pushed the improve_aria_labels branch 2 times, most recently from 7673ded to 49492c0 Compare October 9, 2022 20:35
@nikhilbhatt nikhilbhatt marked this pull request as ready for review October 9, 2022 20:44
julianguyen
julianguyen requested changes Oct 9, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Great work fixing the broken tests 🎉

We have some edge cases with comments which I forgot about 😆 I think after addressing that, we should be good to go 🚀

Thanks for all the updates!

client/app/widgets/Comments/__tests__/Comments.spec.jsx Outdated Show resolved Hide resolved
client/app/widgets/Comments/__tests__/Comments.spec.jsx Outdated Show resolved Hide resolved
client/app/components/Story/StoryActions.jsx Outdated Show resolved Hide resolved
julianguyen
julianguyen requested changes Oct 11, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks so much for the updates 🎉

A few more changes need to be done!

app/views/groups/_info.html.erb Outdated Show resolved Hide resolved
client/app/widgets/Comments/index.jsx Outdated Show resolved Hide resolved
@nikhilbhatt nikhilbhatt marked this pull request as draft October 14, 2022 20:47
@nikhilbhatt nikhilbhatt force-pushed the improve_aria_labels branch 3 times, most recently from 40d5136 to 77a1b18 Compare October 14, 2022 22:13
@nikhilbhatt nikhilbhatt marked this pull request as ready for review October 15, 2022 09:33
julianguyen
julianguyen approved these changes Oct 16, 2022
View reviewed changes
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Fantastic work! Thanks so much for taking this on 🎉 🎊

@julianguyen julianguyen merged commit 5f9be36 into main Oct 16, 2022
@delete-merged-branch delete-merged-branch bot deleted the improve_aria_labels branch October 16, 2022 02:25
0xGhada added a commit that referenced this pull request Oct 16, 2022
@0xGhada
[UPDATE] added story name to aria label (#2158)
bb5eb93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julianguyen julianguyen julianguyen approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@nikhilbhatt @julianguyen