-
Notifications
You must be signed in to change notification settings - Fork 640
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
Generic label for Programs & Events Dashboard #6061
Generic label for Programs & Events Dashboard #6061
Conversation
It looks like this just changes it in all cases. Instead, it needs to switch behavior either based on Course type or based on |
@ragesoss Thanks for the feedback! I’ll update the PR to handle the behavior based on Features.wiki_ed? as you suggested. |
Hi @ragesoss , Is this the program and event dashboard, or the one that appears when we click on the username? |
@bhushan354 "Programs & Events Dashboard" is outreachdashboard.wmflabs.org, one of two production instances of the Dashboard. The other is Wiki Education Dashboard dashboard.wikiedu.org. The two sites have many behavior differences, mainly controlled by the |
template.erb
Outdated
@@ -1 +1,2 @@ | |||
window.stores = <%= JSON.generate(translations) %>; | |||
window.Features = { wiki_ed: <%= Features.wiki_ed?.to_json %> }; |
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.
This is normally handled in the shared _head
haml template. I'm not sure what this .erb file is for but I don't think we use it in production. @TheTrio do you remember what this erb file is for? Something needed for Hot Module Reloading from #5354 ? Or maybe it's just an accident and not used for anything?
@@ -2,10 +2,11 @@ import React from 'react'; | |||
import PropTypes from 'prop-types'; | |||
|
|||
const CourseDetails = ({ courses }) => { | |||
const labelKey = window.Features.wiki_ed ? 'courses.view_page' : 'courses_generic.view_page'; |
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.
window
shouldn't be necessary here, as we normally call Features.wikiEd
anywhere in the JS that needs to switch behavior based on this setting.
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.
Note that we have a helper function that is built to do this kind of switching: CourseUtils.i18n
. It might be that the necessary data is not directly available, but you should check whether the course_string_prefix is part of the JS state already, which will let you use that utility function instead of implementing behavior switch here.
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 have removed that. Thank you for helping me understand this!
Thanks for letting me know , Since the application.yml file has already been set to false, I made the changes while recompiling the webpack. Please let me know if any further enhancements are needed. Thanks ! |
NOTE: Please review the pull request process before opening your first PR: https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/CONTRIBUTING.md#pull-request-process
What this PR does
This PR fixes : #6057
Update link label from 'View Course Page' to 'View Page'
Screenshots
Before:
After:
Open questions and concerns
< anything you learned that you want to share, or questions you're wondering about related to this PR >