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

When "Add component" is clicked change focus (#1963) #2390

Merged

Conversation

Link2Twenty
Copy link
Contributor

Proposed changes

Set focus when "Add component" is clicked #1963

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that the code compiles correctly
  • I have checked that unit test suite passes locally with my changes
  • I have added new tests that cover my changes (where appropriate)
  • I have added documentation (where appropriate)
  • Any dependent changes have already been merged

Change focus to Component search on Component Panel open
@Link2Twenty Link2Twenty changed the title Set focus When "Add component" is clicked change focus (#1963) Jul 16, 2019
@canstudios-nicolaw
Copy link
Contributor

@Link2Twenty Thank you for your continued work, it is appreciated

I can understand why you picked this up, but the "Suggested Next" milestone is a little misleading. Many of the things on this milestone haven't been discussed/thought about in a long time and might not still be desired.

There is going to be a big issue sort/tidy soon to make this clearer, but in the meantime the best issues to work on are the ones on the bugpatch milestone (https://github.com/adaptlearning/adapt_authoring/issues?q=is%3Aopen+is%3Aissue+milestone%3ABugpatch) as these have been most recently sorted as high priority.

I understand there aren't many straightforward beginner issues on the bugpatch milestone right now, so if you'd like to work on an issue that is not on the bugpatch milestone I'd strongly suggest either asking in gitter or tagging myself or @tomgreenfield on the issue to talk about it first. I only suggest this because I don't want you to loose time working on things that turn out to be undesired.

@tomgreenfield I have asked our content team about this, they feel fairly neutral. I think it's acceptable for it to go in because I can't see it making usability worse for people that don't want to use this feature, what do you think?

@canstudios-nicolaw
Copy link
Contributor

I've had some more feedback that this would be useful for some people so I'm going to approve and leave the merging to you if you are happy.

Copy link
Contributor

@tomgreenfield tomgreenfield left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Could you move the focus to the postRender() of the editorPageComponentListView itself? This feels like a better fit than the parent view appending the child and doing a new lookup for the element to focus on.

this.$('input').focus();

Move focus function to `postRender` to ensure render is complete before the focus is set.
Focus function has been moved to editorPageComponentListView's postRender function
@canstudios-nicolaw canstudios-nicolaw added this to the Bugpatch milestone Jul 23, 2019
@Link2Twenty
Copy link
Contributor Author

Could you move the focus to the postRender() of the editorPageComponentListView itself? This feels like a better fit than the parent view appending the child and doing a new lookup for the element to focus on.

I've moved it over to the new file, I've kept the .editor-component-list-sidebar-search-field class in the selector just in case any other inputs are ever added to the sidebar, not that that looks likely to happen.

@canstudios-nicolaw canstudios-nicolaw merged commit 0e80dfc into adaptlearning:release/bugpatch Jul 23, 2019
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