Skip to content

"Graphical Slider Tool" integration into studio - #497

Merged
vasylnakvasiuk merged 3 commits into
masterfrom
alex/graphical_slider_tool_for_studio
Jul 31, 2013
Merged

"Graphical Slider Tool" integration into studio#497
vasylnakvasiuk merged 3 commits into
masterfrom
alex/graphical_slider_tool_for_studio

Conversation

@vasylnakvasiuk

Copy link
Copy Markdown
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

auraz commented Jul 25, 2013

Copy link
Copy Markdown
Contributor

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

@auraz

auraz commented Jul 25, 2013

Copy link
Copy Markdown
Contributor

Please add comment to css why we need it.

@auraz

auraz commented Jul 25, 2013

Copy link
Copy Markdown
Contributor

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

@vasylnakvasiuk

Copy link
Copy Markdown
Contributor Author

@auraz, done.

@chrisndodge

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@vasylnakvasiuk

Copy link
Copy Markdown
Contributor Author

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

@chrisndodge

Copy link
Copy Markdown
Contributor

Tried branch. Looks good.

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

Copy link
Copy Markdown
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
Copy Markdown
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.

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

@cpennington, done python style fix.

@cpennington

Copy link
Copy Markdown
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