Skip to content
This repository has been archived by the owner on Jul 28, 2020. It is now read-only.

Fix #113 issue: mod_quickicon missing strings #113 #153

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

Fix #113 issue: mod_quickicon missing strings #113 #153

wants to merge 1 commit into from

Conversation

davidnguyen-joomlashine
Copy link

Pull Request for Issue #113

Summary of Changes

Fix JText string

Testing Instructions

Expected result

Actual result

Documentation Changes Required

brianteeman
brianteeman reviewed Dec 9, 2019
View reviewed changes
Copy link
Contributor

@brianteeman brianteeman left a comment

Choose a reason for hiding this comment

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

This is obviously not correct. Why would you have text that is set be displayed only to screen readers that is identical to text that is set to be hidden from screen readers?

@davidnguyen-joomlashine
Copy link
Author

@brianteeman I checked and see that "sr-only" presents actions: View all Users, View all Menu Items View all Articles and View all Modules, so I thought it makes sense if it is identical to text that be hidden from screen readers. Please let me know if I missed something. Thank you so much

@brianteeman
Copy link
Contributor

Think about it for a second. Why would you hide text and then choose to display the exact same text

@davidnguyen-joomlashine
Copy link
Author

@brianteeman I think it is not necessary, so it should be removed out. What do you think of my idea.

Thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brianteeman brianteeman brianteeman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidnguyen-joomlashine @brianteeman @giangnd-bb