Tapestry
  1. Tapestry
  2. TAPESTRY-199

Addition of a simple way to get component CSS stylesheets into an overall page

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0, 4.0.2
    • Fix Version/s: 4.1.1
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All

      Description

      As I understand it, if a component requires a CSS stylesheet, you either have to put <style> commands
      in the generated HTML or you have to change the global page when you add the component.

      The first method means that the user would find it hard to override or change the component, as well
      as increased overhead and blowing out the browser cache.

      The second method breaks component isolation. Essentially, the main page for an app suddenly must
      be concerned about the styles used by various subcomponents.

      I believe we would be well served by enhancing the @Shell component to automatically pick up the
      stylesheets declared as needed in subcomponents. The situation is similar to how @Body picks up
      @Script components and tucks them into just one Javascript block.

      This would require a way for a component to register the stylesheets it uses. I would expect a tag in
      the jwc that sets the IAsset stylesheet/stylesheets, as then the top level component could just look for
      those assets in the contained items.

      I would expect, then, that the top level stylesheet/stylesheets passed into the @Shell component would
      be augmented silently. There is an argument for a "useComponentStylesheets" attribute that is by
      default true to control this behavior.

      The result would be that a user would not need to know that a component has added a stylesheet,
      because it would just work. It would be snazzy if the addition was smart enough to only include a
      stylesheet onec if there were multiple instances of the same component, or if there were two
      components using the same stylesheet.

        Issue Links

          Activity

          Scott Ellsworth created issue -
          Hide
          Filip S. Adamsen added a comment -

          I thought about something similar earlier today. My idea, which is very similar
          to yours, is to create a @Style component taking one required parameter (an
          IAsset, I suppose), the style sheet to be used, and an optional parameter to
          control the media setting. This component, coupled with some logic in the
          @Shell component, would allow us to specify stylesheets without hassle, just as
          you described.

          I must say that I prefer the component-based approach to relying on predefined
          page properties. I believe it would be trivial to implement. Off course the
          stylesheet(s) specified in the @Shell should be added to the response as the
          last one(s), in order to satisfy the cascade. This way, we'd be able to
          override the style sheets specified by the components in an easy way.

          Show
          Filip S. Adamsen added a comment - I thought about something similar earlier today. My idea, which is very similar to yours, is to create a @Style component taking one required parameter (an IAsset, I suppose), the style sheet to be used, and an optional parameter to control the media setting. This component, coupled with some logic in the @Shell component, would allow us to specify stylesheets without hassle, just as you described. I must say that I prefer the component-based approach to relying on predefined page properties. I believe it would be trivial to implement. Off course the stylesheet(s) specified in the @Shell should be added to the response as the last one(s), in order to satisfy the cascade. This way, we'd be able to override the style sheets specified by the components in an easy way.
          Hide
          Filip S. Adamsen added a comment -
              • Bug 30454 has been marked as a duplicate of this bug. ***
          Show
          Filip S. Adamsen added a comment - Bug 30454 has been marked as a duplicate of this bug. ***
          Hide
          Filip S. Adamsen added a comment -

          Created an attachment (id=12328)
          Unified diff containing all changes needed for the Style component to function properly.

          Show
          Filip S. Adamsen added a comment - Created an attachment (id=12328) Unified diff containing all changes needed for the Style component to function properly.
          Serge Knystautas made changes -
          Field Original Value New Value
          issue.field.bugzillaimportkey 30393 22706
          Hide
          Markus Joschko added a comment -

          Just wanted to stress that I think it would be very usefull to apply this patch to the codebase.
          Having a way to add CSS within a component to the surrounding page is really crucial for todays webcomponents.

          Show
          Markus Joschko added a comment - Just wanted to stress that I think it would be very usefull to apply this patch to the codebase. Having a way to add CSS within a component to the surrounding page is really crucial for todays webcomponents.
          Hide
          Andreas Andreou added a comment -
          Show
          Andreas Andreou added a comment - A possible solution to this for Tapestry 3 version is described in http://andyhot.di.uoa.gr/blojsom/blog/default/TapFX/2005/08/19/Allowing_Tapestry_components_to_contribute_CSS.html
          Jesse Kuhnert made changes -
          Assignee Tapestry Developer List [ tapestry-dev@jakarta.apache.org ] Jesse Kuhnert [ jkuhnert ]
          Andreas Andreou made changes -
          Fix Version/s 4.0.3 [ 12310994 ]
          Fix Version/s 3.0.5 [ 12310940 ]
          Bugzilla Id 30393
          Priority Blocker [ 1 ]
          Affects Version/s 4.0.2 [ 12310870 ]
          Jesse Kuhnert made changes -
          Fix Version/s 4.1 [ 12310632 ]
          Fix Version/s 3.0.5 [ 12310940 ]
          Bugzilla Id 30393
          Fix Version/s 4.0.3 [ 12310994 ]
          Hide
          Norbert Sándor added a comment -

          I think the attached patch won't work with 4.1 because of TAPESTRY-1012.
          I haven't tested, but my old 4.0 solution works similarly...

          Show
          Norbert Sándor added a comment - I think the attached patch won't work with 4.1 because of TAPESTRY-1012 . I haven't tested, but my old 4.0 solution works similarly...
          Hide
          Jesse Kuhnert added a comment -

          Changing status from "blocker" to major.

          Show
          Jesse Kuhnert added a comment - Changing status from "blocker" to major.
          Jesse Kuhnert made changes -
          Priority Blocker [ 1 ] Major [ 3 ]
          Bugzilla Id 30393
          Jesse Kuhnert made changes -
          Fix Version/s 4.1 [ 12310632 ]
          Fix Version/s 4.1.1 [ 12312021 ]
          Andreas Andreou made changes -
          Assignee Jesse Kuhnert [ jkuhnert ] Andreas Andreou [ andyhot ]
          Andreas Andreou made changes -
          Link This issue is blocked by TAPESTRY-1012 [ TAPESTRY-1012 ]
          Andreas Andreou made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Andreas Andreou added a comment -

          Fixed by new Relation component and related changes in Shell.java

          Show
          Andreas Andreou added a comment - Fixed by new Relation component and related changes in Shell.java
          Andreas Andreou made changes -
          Resolution Fixed [ 1 ]
          Status In Progress [ 3 ] Resolved [ 5 ]
          Hide
          Jesse Kuhnert added a comment -

          I don't see any documentation or unit tests covering this new feature.

          Not that I'm not completely grateful and happy that you added it in, but a feature without documentation might as well not exist.

          Show
          Jesse Kuhnert added a comment - I don't see any documentation or unit tests covering this new feature. Not that I'm not completely grateful and happy that you added it in, but a feature without documentation might as well not exist.
          Jesse Kuhnert made changes -
          Status Resolved [ 5 ] Reopened [ 4 ]
          Resolution Fixed [ 1 ]
          Jesse Kuhnert made changes -
          Link This issue is duplicated by TAPESTRY-1080 [ TAPESTRY-1080 ]
          Hide
          Andreas Andreou added a comment -

          Docs added.

          Show
          Andreas Andreou added a comment - Docs added.
          Andreas Andreou made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Closed [ 6 ]
          Andreas Andreou made changes -
          Link This issue is duplicated by TAPESTRY-1080 [ TAPESTRY-1080 ]
          Mark Thomas made changes -
          Workflow jira [ 32701 ] Default workflow, editable Closed status [ 12568970 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12568970 ] jira [ 12592018 ]

            People

            • Assignee:
              Andreas Andreou
              Reporter:
              Scott Ellsworth
            • Votes:
              9 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development