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

Added deliteful/ScrollableContainer #20

Closed

Conversation

AdrianVasiliu
Copy link
Contributor

Contains:

  • A container widget with scrolling capabilities (using delite/Scrollable).
  • Intern unit tests.
  • Functional tests.

Notes:

  • Superseeds deliteful/ScrollableContainer #11.
  • Layout troubles of the functional tests on IE (not only IE9 but also IE10) are related with flexbox/LinearLayout issues on IE.

@AdrianVasiliu
Copy link
Contributor Author

Among other changes, this PR addresses Bill's comments: inheritance from Widget now unnecessary, and reuse of the shared test cases from delite.

@pruzand
Copy link
Member

pruzand commented Jan 31, 2014

Seems this PR is now ready to be merged. If no one objects, Adrian, can you merge it ?

@wkeese
Copy link
Member

wkeese commented Jan 31, 2014

I'm confused about the tests. You have 3 files in tests/functional yet they are all HTML files; there's no functional automated test? (Having said that, I'm not sure it's practical to test scrolling via webdriver.)

@AdrianVasiliu
Copy link
Contributor Author

Yes, they are manual tests, complementing the automatic unit tests. I could have added some minimalist automatic testing but as you said it's not quite practical to test scrolling via webdriver. Two main reasons: the interaction is browser-dependant (mouse interaction with the scrollbar on non-touch devices, scroll gesture on touch devices), and checking the quality/performance of the scrolling is hard to automate.

Since the scrolling is now done by the browser itself, and we don't want to test the browser feature but our stuff around it, I would think the automatic unit testing would be good enough for now, but of course we might improve it later.

@wkeese
Copy link
Member

wkeese commented Jan 31, 2014

It just seems strange to put manual tests in test/functional. Was that what we agreed on? I thought they were supposed to go in samples/ but I can't remember for sure.

@cjolif
Copy link
Contributor

cjolif commented Jan 31, 2014

The agreement was to have no manual tests ;) Only intern functional and unit.

And samples of usage must indeed be in samples.

@AdrianVasiliu
Copy link
Contributor Author

Well, from "delite(ful)/tests/functional/MyClass.html (even if that is used standalone this is still some kind of functional tests not a unit test for sure)" in ibm-js/sdk#2 I understood that this is "tolerated".
Now, it could also go to sample but it depends on the criteria to know what qualifies as a sample vs a manual functional test. Did we state that somewhere?

@cjolif
Copy link
Contributor

cjolif commented Jan 31, 2014

I said "used standalone" which does not state it is its sole purpose, my idea was that it can be used standalone but my feeling is that there is always something to test automatically on top of that? Also I think there is a value making sure an interaction is always automatically performed on a widget. This makes sure at least we are not breaking on interaction (for example). That said I guess this can wait post merge.

@AdrianVasiliu
Copy link
Contributor Author

Okay, so if nobody objects I would improve that post merge, as Christophe said.

@AdrianVasiliu
Copy link
Contributor Author

Committed in master: 36ad53b

wkeese added a commit that referenced this pull request Jan 29, 2015
This means that buildRendering() runs before any user the user defined parameters
are available.

This change is necessary to be forward compatible with browsers that implement
document.register() natively, because in that case programmatic creation (w/out
the syntactic sugar) will be:

var node = document.createElement("dui-slider");
node.max = 10;

Other changes:

- Removed runAfterRender() helper.
- Added preCreate() lifecycle method that executes before buildRendering().
- Fixed a few widgets that were depending on constructor() getting called;
  constructor() doesn't get called anymore with custom elements.
- Removed widgetId attribute setting on nodes.

Open issues:

- _Invalidating is presumably emitting unwanted events for changes that are due to
  initialization parameters; it should only send events for changes after this._started.
- Specifying event handlers declaratively (ex: <dui-button onfoo=...> for custom events
  likely isn't working.

Fixes #20.
wkeese added a commit that referenced this pull request Feb 6, 2015
This means that buildRendering() runs before any user the user defined parameters
are available.

This change is necessary to be forward compatible with browsers that implement
document.register() natively, because in that case programmatic creation (w/out
the syntactic sugar) will be:

var node = document.createElement("dui-slider");
node.max = 10;

Other changes:

- Removed runAfterRender() helper.
- Added preCreate() lifecycle method that executes before buildRendering().
- Fixed a few widgets that were depending on constructor() getting called;
  constructor() doesn't get called anymore with custom elements.
- Removed widgetId attribute setting on nodes.

Open issues:

- _Invalidating is presumably emitting unwanted events for changes that are due to
  initialization parameters; it should only send events for changes after this._started.
- Specifying event handlers declaratively (ex: <dui-button onfoo=...> for custom events
  likely isn't working.

Fixes #20.
wkeese added a commit to wkeese/deliteful that referenced this pull request Mar 12, 2015
This means that buildRendering() runs before any user the user defined parameters
are available.

This change is necessary to be forward compatible with browsers that implement
document.register() natively, because in that case programmatic creation (w/out
the syntactic sugar) will be:

var node = document.createElement("dui-slider");
node.max = 10;

Other changes:

- Removed runAfterRender() helper.
- Added preCreate() lifecycle method that executes before buildRendering().
- Fixed a few widgets that were depending on constructor() getting called;
  constructor() doesn't get called anymore with custom elements.
- Removed widgetId attribute setting on nodes.

Open issues:

- _Invalidating is presumably emitting unwanted events for changes that are due to
  initialization parameters; it should only send events for changes after this._started.
- Specifying event handlers declaratively (ex: <dui-button onfoo=...> for custom events
  likely isn't working.

Fixes ibm-js#20.
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