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.

Sign up for GitHub

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

Jump to bottom

Miscellaneous fixes #103

Merged
merged 3 commits into from
Sep 22, 2016
Merged

Miscellaneous fixes #103

merged 3 commits into from
Sep 22, 2016

Conversation

pomegranited
Copy link
Contributor

@pomegranited pomegranited commented Sep 15, 2016 •
edited
Loading

Description

  • Consistent capitalization in the Studio editor
  • Remove dangerous recommendation to delete from the top of the editor
  • Stop large images from overflowing feedback popups in Studio
  • Also includes changes from PR RTL fixes #102, see comment.

Dependencies

None

JIRA

Sandbox URLs

Latest commit

Testing instructions

  1. Go to the preconfigured block on Studio, and log in as staff@example.com.
  2. Edit the block.
  3. Notice that there is no message on the top advising authors to delete the block.
  4. Read through all items, pressing "Continue" until the last page, and notice that all item headers (such as "Introductory feedback", or "Success feedback") follow the same capitalization convention.
  5. Notice that the success feedbacks on this instance contain a 700x700 placeholder image. Close the edit box, and place an item correctly. The image should be properly contained in the popup, as seen in the screenshot below.

Screenshots

  1. What a large image should look like in a success feedback popup, in Studio or the LMS:
    screenshot1
  2. Consistent capitalization, in Studio:
    screenshot2
  3. The removed message from the top of the edit form:
    screenshot3

Reviewers

arbrandes and others added 2 commits September 14, 2016 19:34
@arbrandes
RTL fixes
fe49f58 
Introduce proper right-to-left support for the following:

* Alignment of overall feedback message icons and text
* Alignment and borders of sidebar buttons
* Margin of the "You have used X attempts" message
@pomegranited
Merge pull request open-craft#12 from arbrandes/OC-1859
d795e3c 
RTL fixes
@openedx-webhooks
Copy link

Thanks for the pull request, @pomegranited! It looks like you're a member of a company that does contract work for edX. If you're doing this work as part of a paid contract with edX, you should talk to edX about who will review this pull request. If this work is not part of a paid contract with edX, then you should ensure that there is an OSPR issue to track this work in JIRA, so that we don't lose track of your pull request.

To automatically create an OSPR issue for this pull request, just visit this link: https://openedx-webhooks.herokuapp.com/github/process_pr?number=103&repo=edx-solutions%2Fxblock-drag-and-drop-v2

@arbrandes
Miscellaneous fixes
e5eca5a 
* Consistent capitalization in the Studio editor
* Remove dangerous recommendation to delete from the top of the editor
* Stop large images from overflowing feedback popups in Studio
cahrens
cahrens reviewed Sep 15, 2016
View reviewed changes
Copy link

@cahrens cahrens left a comment

Choose a reason for hiding this comment

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

I have a question about the CSS, but the other changes look fine.

drag_and_drop_v2/public/css/drag_and_drop.css
@@ -307,34 +307,70 @@


/* Font Awesome icons have different width - margin-right tries to compensate it */
.xblock--drag-and-drop .feedback p:before {
.ltr .xblock--drag-and-drop .feedback p:before,
Copy link

Choose a reason for hiding this comment

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

@alisan617 or @bjacobel, is there a better way to handle rtl/ltr? I don't recall seeing this pattern anyplace else.

Copy link

Choose a reason for hiding this comment

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

@cahrens unfortunately this is css not sass. Otherwise, could import bi-app sass.

@cahrens cahrens mentioned this pull request Sep 15, 2016
Closed
4 tasks
@cahrens
Copy link

cahrens commented Sep 16, 2016

👍

@pomegranited
Copy link
Contributor Author

Thanks, @cahrens !

@marcotuts, could you have a look at this PR when you get a chance?

@sstack22
Copy link

@pomegranited - 👍 from a product standpoint.

@pomegranited
Copy link
Contributor Author

Thank you @sstack22 ! Merging now.

@pomegranited pomegranited merged commit 07add13 into openedx:master Sep 22, 2016
@pomegranited
Copy link
Contributor Author

@arbrandes All merged.. Thank you, and sorry for my mistakes on this one!

@arbrandes arbrandes deleted the OC-1883 branch September 27, 2016 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cahrens cahrens cahrens left review comments

@alisan617 alisan617 alisan617 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pomegranited @openedx-webhooks @cahrens @sstack22 @alisan617 @arbrandes