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

issue-1469: extended tutor #61

Merged
merged 19 commits into from Oct 13, 2021
Merged

issue-1469: extended tutor #61

merged 19 commits into from Oct 13, 2021

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Mar 15, 2017

fixes: adaptlearning/adapt_framework#1469

Added

  • overlay and inline feedback options
  • option of a notify text button at the bottom rather than a close button top right
  • _classes to allow classes to be added to the feedback

Updated

@oliverfoster
Copy link
Member Author

oliverfoster commented Mar 15, 2017

i need help with schemas on this one. tutor + feedback are quite disjointed.
see bottom of the issue for my thoughts on supporting the schema.

@oliverfoster
Copy link
Member Author

@tomgreenfield is taking this one for investigation

@tomgreenfield tomgreenfield marked this pull request as draft June 18, 2021 10:12
@oliverfoster oliverfoster changed the base branch from master to newmaster June 22, 2021 18:05
@oliverfoster
Copy link
Member Author

@guywillis can you have a look at the styling
@oliverfoster can you do the accessibility testing

@oliverfoster oliverfoster added this to New in adapt_framework: The TODO Board via automation Jun 29, 2021
@oliverfoster oliverfoster moved this from New to Needs reviewing in adapt_framework: The TODO Board Jun 29, 2021
@tomgreenfield
Copy link
Contributor

tomgreenfield commented Jul 28, 2021

See questionView.js#L300 for the line in core where we could do with a { preventScroll: true }, to stop jerking on reset. done

@guywillis
Copy link
Contributor

guywillis commented Aug 3, 2021

Some things to consider:

  1. "_type": "none", - Turns off the feedback but leaves the show feedback button active. Should this be allowed?
  2. "_type": "inline", + "_hasNotifyBottomButton": false, - Hide feedback button is still at the bottom of the inline feedback
  3. "_type": "overlay", + "_hasNotifyBottomButton": true, - Hide feedback button is still the close icon in the top right of the overlay feedback

As an aside:

  1. Is anyone aware of a method or config option to force buttons to be full width?

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oliverfoster
Copy link
Member Author

oliverfoster commented Aug 3, 2021

Some things to consider:

  1. "_type": "none", - Turns off the feedback but leaves the show feedback button active. Should this be allowed?
  2. "_type": "inline", + "_hasNotifyBottomButton": false, - Hide feedback button is still at the bottom of the inline feedback
  3. "_type": "overlay", + "_hasNotifyBottomButton": true, - Hide feedback button is still the close icon in the top right of the overlay feedback

As an aside:

  1. Is anyone aware of a method or config option to force buttons to be full width?
  1. Yes, as question has _canShowFeedback and a different extension may take the feedback for this item... probably? It's an edge case I'll admit.
  2. Would you prefer "_hasBottomButton": true, ?
  3. ^
  4. In what context?

@oliverfoster
Copy link
Member Author

@guywillis could you re-review please? / answer the above questions

less/tutor.less Outdated Show resolved Hide resolved
Co-authored-by: eleanor-heath <61159216+eleanor-heath@users.noreply.github.com>
@oliverfoster oliverfoster merged commit 2772f74 into master Oct 13, 2021
adapt_framework: The TODO Board automation moved this from Needs reviewing to Needs releasing Oct 13, 2021
@oliverfoster oliverfoster deleted the issue/1469 branch October 13, 2021 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Extend tutor
6 participants