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

Resolves #2395 and #2389 - Capture child views #2413

Merged
merged 9 commits into from
Jun 3, 2019
Merged

Conversation

danielstorey
Copy link
Member

@danielstorey danielstorey commented Apr 10, 2019

#2395

  • Capture child views
  • Provide findViewByModelId api

#2389

  • Remove rtl prepend

@oliverfoster oliverfoster changed the title Resolves #2395 and #2389 Resolves #2395 and #2389 - Capture child views Apr 15, 2019
@moloko
Copy link
Contributor

moloko commented May 2, 2019

Useful!

Two things:

  1. Any chance you could add a brief bit of documentation about these to the Adapt API Wiki page?
  2. I can totally see why you did it - but it does seem a little odd to have some code that addresses a totally separate issue (RTL layout issues #2389) in here. It also doesn't completely fix the issue (in that although the DOM order is now correct the layout - in medium/large screen size - isn't. Using block b-05 as an example, when in RTL mode, the text component c-05 should be on the right and the graphic component on the left). I wonder if it would be better to do this as a separate PR that includes the layout fix? Not sure whether this is best done by re-assigning the .component-left and .component-right classes or by amending those classes for RTL mode. Suggestions welcome...!

Copy link
Member

@brian-learningpool brian-learningpool left a comment

Choose a reason for hiding this comment

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

👀

@oliverfoster oliverfoster merged commit 667adf7 into master Jun 3, 2019
@oliverfoster oliverfoster deleted the issue/2395 branch June 3, 2019 12:28
@oliverfoster
Copy link
Member

henrietta-k pushed a commit to henrietta-k/adapt-learning-wiki that referenced this pull request Apr 7, 2024
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.

6 participants