Skip to content

Conversation

@manu-d
Copy link
Contributor

@manu-d manu-d commented Mar 12, 2018

No description provided.

@manu-d manu-d requested a review from xaun March 12, 2018 17:19
Copy link
Contributor

@xaun xaun 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 this, looks like it needs to be scrollable, and mainly I think the requirements need to be reviewed with @cesar-tonnoir (based on the un-used binding that were specified in the JIRA).

I need to confirm some things with Cesar myself, as currently I see two use cases for the Layouts.

  1. A developer is building a custom widget template, and wants to leverage a component that handles the rendering of the layout (what you have done for "days between visits" widget).

  2. No custom template is defined for a v2 bolt widget, and a generic template is to be loaded. See this PR of mine #501, which adds a table layout, which can be loaded by #484.

I think my PR is missing use case 1, and your PR is missing use case 2.

restrict: 'E',
scope: {
content: '='
timePeriodEnabled: '=?'
Copy link
Contributor

Choose a reason for hiding this comment

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

timePeriodEnabled, selectorList, and selectorUid isn't used anywhere.

@@ -0,0 +1,10 @@
.analytics .layouts.figure {
Copy link
Contributor

Choose a reason for hiding this comment

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

No styles are being applied to indicate this is a column or scrollable as specified in the JIRA.
In screenshot below I have added entries and they just extend the height of the widget.
screen shot 2018-03-19 at 11 23 56

@cesar-tonnoir cesar-tonnoir added this to the v1.8.0 milestone Apr 11, 2018
@cesar-tonnoir cesar-tonnoir modified the milestones: v1.8.0, v1.9.0 May 24, 2018
@agranado2k agranado2k modified the milestones: v1.9.0, LMI Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants