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
Add Datasets index page to Levelbuilder #33140
Conversation
<ul> | ||
{this.props.datasets.map(dataset => ( | ||
<li key={dataset}> | ||
<a href={`/datasets/${dataset}`}>{dataset}</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For accessibility, should this include a title attribute? i.e. title="{dataset} dataset" --- display as ---> "Dogs dataset"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also, is levelbuilder within our accessibility goals?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that would make this more accessible (header on the page is Datasets and the links are /datasets/{name}
so I think it is already contextually clear that the list contains the names of datasets).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -5,8 +5,13 @@ class DatasetsController < ApplicationController | |||
before_action :initialize_firebase | |||
authorize_resource class: false | |||
|
|||
LIVE_DATASETS = ['Daily Weather', 'Top 200 USA', 'Top 200 Worldwide', 'Viral 50 USA', 'Viral 50 Worldwide'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to make this localized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean localized? These are the names of the live tables, which is not translated/translatable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking: Is translating these titles something we'd ever need to worry about? Or if we wanted to internationalize this feature would we instead import data tables with different languages?
Part 1
Fetches the list of tables according to
cdo-v3-shared/v3/channels/shared/counters/tables
and displays them as a list.Links
Testing story
Reviewer Checklist: