feat(search/svelte): Support more file icons#63181
Conversation
There was a problem hiding this comment.
Just for my education: it's okay to add aria-hidden to this because the wrapping button has an aria-label on it, so the actual contents of the button are irrelevant to the screenreader, yes?
There was a problem hiding this comment.
Normally "images" are part of the accessibility tree and will be "read" by screenreaders. E.g. without aria-hidden it shows up like this in the accessibility tree:
With the attribute the "graphics-document" just isn't there. Often icons are just decorative in which case they should be hidden to screenreaders. And yes, the button already has a label and therefore the content is irrelevant.
Alternatively we could have add text inside the button that is only visible to screenreaders, but aria-label seems simpler.
Until this commit both the React and Svelte apps have used the same code to determine the correct file icon. That code however still held separate logic for the two apps with the result that our file icon set for the Svelte app was a lot smaller. This commit separates the logic and now makes use of `unplugin-icons` in the Svelte app. I tried to use `unplugin-icons` in the React so that the code could continued to be shared but I couldn't get it to work.
6b9d11f to
81b8ae1
Compare
Closes SRCH-452
Until this commit both the React and Svelte apps have used the same code to determine the correct file icon. That code however still held separate logic for the two apps with the result that our file icon set for the Svelte app was a lot smaller.
This commit separates the logic and now makes use of
unplugin-iconsin the Svelte app.I tried to use
unplugin-iconsin the React so that the code could continued to be shared but I couldn't get it to work.Note that I didn't use the exact same icons as in the React app. I wanted to minimize the number of different icon sets we are using.
Test plan
Builds without error, manual testing.