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.

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

"Graphical Slider Tool" integration into studio #497

Merged
merged 3 commits into from
Jul 31, 2013

Conversation

vasylnakvasiuk
Copy link
Contributor

We add new field "data" and remove "render" and "configuration", but we don't add backward compatibility functionality, cause this "blade" have been never used before with Mongo, so we haven't any stored data and everything will work fine.
@auraz, @cpennington, please review

@ghost ghost assigned cpennington Jul 25, 2013
@auraz
Copy link
Contributor

auraz commented Jul 25, 2013

Please remove cms/templates/widgets/graphical_slider_tool-edit.html from PR

@auraz
Copy link
Contributor

auraz commented Jul 25, 2013

Please add comment to css why we need it.

@auraz
Copy link
Contributor

auraz commented Jul 25, 2013

Good to go, as soon as you will address all comments.

@vasylnakvasiuk
Copy link
Contributor Author

@auraz, done.

@chrisndodge
Copy link
Contributor

Sorry, 2 failing tests here:

xmodule.tests.test_export.RoundTripTestCase.test_graphicslidertool_roundtrip
xmodule.tests.test_import.ImportTestCase.test_graphicslidertool_import

@vasylnakvasiuk
Copy link
Contributor Author

Ouch... I'll fix them. My fault.

@vasylnakvasiuk
Copy link
Contributor Author

@chrisndodge, I've fixed tests and solved pep8 issues.

@chrisndodge
Copy link
Contributor

Tried branch. Looks good.

@talbs does one of the UI people want to review the CSS edits?

resource_string(__name__, 'js/src/graphical_slider_tool/gst.js')

]
'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee')],
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than setting these here, I think it would be better to have something like:

@property
def configuration(self):
    return stringify_children(html.fromstring(self.data).xpath('configuration')[0])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cpennington, thanks for your suggestions. Done.

@property
def configuration(self):
return stringify_children(
html.fromstring(self.data).xpath('configuration')[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: per https://edx-wiki.atlassian.net/wiki/display/ENG/Python+Guidelines?src=search, the final ) should be on the next line.

@vasylnakvasiuk
Copy link
Contributor Author

@cpennington, done python style fix.

@cpennington
Copy link
Contributor

👍

vasylnakvasiuk added a commit that referenced this pull request Jul 31, 2013
"Graphical Slider Tool" integration into studio
@vasylnakvasiuk vasylnakvasiuk merged commit 1233d73 into master Jul 31, 2013
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
@jzoldak jzoldak deleted the alex/graphical_slider_tool_for_studio branch May 5, 2014 14:53
diegomillan pushed a commit to eduNEXT/edx-platform that referenced this pull request Sep 14, 2016
…cs_list

Fix metrics tab downloads for split mongo courses.
xavierchan added a commit to xavierchan/edx-platform-1 that referenced this pull request Feb 17, 2020
yoann-mroz pushed a commit to weuplearning/edx-platform that referenced this pull request Nov 30, 2020
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
* [feat] Add ID verification Alert to course home

if a user has a verified seat, but is in the unverified certificate
status state, the certificateStatusAlert will now show a message letting
the learner know they need to verify in order to earn a certificate.

This does not remove the message about the verification deadline in the
right sidebar of the course home.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants