You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Chris Geer <ch...@cxtsoftware.com> on 2012/04/26 01:47:10 UTC

Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4880/
-----------------------------------------------------------

Review request for rave.


Summary
-------

I pulled the "menubar" code out of page.jsp and put it into common/menubar.jsp to support easier customization for users. I added an optional attribute to the base-layout to support this additional field and added it to the tile-def for standard pages.

Something similar might be done for profile and store in the future but they can't use this exact JSP since their menu options differ from normal pages.


Diffs
-----

  /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp PRE-CREATION 
  /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1330602 
  /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330602 
  /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml 1330602 

Diff: https://reviews.apache.org/r/4880/diff


Testing
-------

Ran portal and witnessed no problems.


Thanks,

Chris


Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
Jasha,

Short term as in hard coded nav items, not DB driven ;)

I recognize that the JCR proposal will change how content is stored but if
won't inherently solve the splitting of the pages. I'm hoping what we do
here (and what has already been done with the admin pages) will
be reusable with the new JCR backend. If not, let's discuss that so we
don't do a bunch

One mechanic that I guess I could for see changing is the use of the
standard tile-defs. Is there any more detail into how the new backend will
stitch pages together from it's pieces? I assume we will ultimately have to
migrate every page to a template based page. Is that correct?

Chris

On Fri, Apr 27, 2012 at 8:00 AM, Jasha Joachimsthal <
j.joachimsthal@onehippo.com> wrote:

> No idea who made the navigation in the admin interface, but it looks like a
> great short term solution ;)
> To be serious: there is already a proposal to split up the way pages are
> built up [1]. It won't be available this release or the next, but in a few
> months' time, pages will be built up in a modular way.
>
> [1] http://markmail.org/thread/npuldma3yxibourj
>
> Regards,
>
> Jasha
>
> On 26 April 2012 23:27, Chris Geer <ch...@cxtsoftware.com> wrote:
>
> > Stan,
> >
> > I agree with you. I think I have a short term and a long term solution.
> The
> > short term solution is to reuse the same concept that is used for the
> admin
> > page right now where the model is given NavigationItems during page
> > creation. Right now those are hardcoded in the controller. I'm going to
> > replicate that for page, store and whatnot. I'm also going to add another
> > task that will be to load those dynamically from the DB. That way plugins
> > could add entries for each page type to the DB to get new nav items. And
> if
> > a user wants to override the navbar they can just overwrite header.tag
> > (which I plan on renaming since it's confusing with include/header.jsp).
> >
> > Chris
> >
> > On Thu, Apr 26, 2012 at 2:14 PM, Drozdetski, Stan A.
> > <dr...@mitre.org>wrote:
> >
> > > I like the spirit of your second suggestion - just not sure about the
> > > implementation.
> > >
> > > (I don't mean "not sure because I don't like it" - as a front-end guy,
> I
> > > just don't know the right way to implement what you're asking for. But
> > > getting this right is surprisingly critical for extensibility.)
> > >
> > > In the past, we've discussed the ability to extend Rave using
> > > plugins/extensions/whatever. It would be mightily useful if a plugin is
> > > able to dynamically add an item to a specified menu. If DB is the place
> > to
> > > do it, I'm all for it.
> > >
> > > The alternative - file replacement - is limiting when it comes to
> > > customizing your instance, and then trying to upgrade to a new version
> of
> > > Rave.
> > >
> > > (Background - the platform that I'm familiar with, Elgg, first allowed
> > > plugins a programmatic way to append menu items. That quickly became
> hard
> > > to manage, since some plugins wanted to override certain options added
> by
> > > other plugins. In one of the recent releases, the menus changed - now,
> a
> > > plugin can register and unregister menu items. You also have a
> provision
> > to
> > > specify the sort order for menu items. Saved us a lot of pain that
> would
> > > otherwise be caused by file replacement.)
> > >
> > > Stan Drozdetski
> > > MITRE
> > >
> > > -----Original Message-----
> > > From: Chris Geer [mailto:chris@cxtsoftware.com]
> > > Sent: Thursday, April 26, 2012 4:19 PM
> > > To: dev@rave.apache.org
> > > Subject: Re: Review Request: Separates the menubar from the page.jsp
> into
> > > a standalone jsp for easy customization
> > >
> > > In regards to this I've got a couple of ideas (limited to navbar for
> the
> > > moment):
> > >
> > > 1) The simplest approach is to define multiple navbar.tag files (i.e.
> > > userpage_navbar.tag, store_navbar.tag) and that way each jsp file can
> > > properly incorporate the correct navbar layout they want. Same thing
> > could
> > > be done with tiles.
> > > 2) Another approach, like you suggested, is make them dynamic based on
> > some
> > > configuration. I like this approach but I'm not sure how we would drive
> > it
> > > out of the DB. Page_template would be the perfect place but most pages
> > > aren't based on page templates at the moment. In fact, store.jsp
> doesn't
> > > look like it pulls any data from the DB. I guess we could build it
> where
> > it
> > > was based on page_type and just have static values from some page
> types.
> > So
> > > maybe, STORE, USER, ADMIN, PERSON_PROFILE?
> > >
> > > If it was DB driven I envision a navitems table with the following
> > columns:
> > > id, page_type, menu_name, menu_item_name, menu_item_url,
> menu_item_icon.
> > > (being to have multiple groups is important to us since we want to have
> > the
> > > items split between two dropdown menus)
> > >
> > > Parent User Story: https://issues.apache.org/jira/browse/RAVE-585
> > >
> > > Chris
> > >
> > > On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:
> > >
> > > >
> > > > -----------------------------------------------------------
> > > > This is an automatically generated e-mail. To reply, visit:
> > > > https://reviews.apache.org/r/4880/#review7265
> > > > -----------------------------------------------------------
> > > >
> > > >
> > > > Thanks for the patch.  A couple of issues though:
> > > >
> > > > 1) This only takes into account the portal pages.  Menus on other
> pages
> > > > are rendered differently.  What would be the best approach is to
> have a
> > > > single menubar with navigation contributions based on page template
> and
> > > > render out the nav elements dynamically. This approach can then be
> > > extended
> > > > to allow menu item contributions from other sources.
> > > >
> > > > 2) This needs a JIRA ticket for us to Apply it :)
> > > >
> > > > - mfranklin
> > > >
> > > >
> > > > On 2012-04-25 23:47:10, Chris Geer wrote:
> > > > >
> > > > > -----------------------------------------------------------
> > > > > This is an automatically generated e-mail. To reply, visit:
> > > > > https://reviews.apache.org/r/4880/
> > > > > -----------------------------------------------------------
> > > > >
> > > > > (Updated 2012-04-25 23:47:10)
> > > > >
> > > > >
> > > > > Review request for rave.
> > > > >
> > > > >
> > > > > Summary
> > > > > -------
> > > > >
> > > > > I pulled the "menubar" code out of page.jsp and put it into
> > > > common/menubar.jsp to support easier customization for users. I added
> > an
> > > > optional attribute to the base-layout to support this additional
> field
> > > and
> > > > added it to the tile-def for standard pages.
> > > > >
> > > > > Something similar might be done for profile and store in the future
> > but
> > > > they can't use this exact JSP since their menu options differ from
> > normal
> > > > pages.
> > > > >
> > > > >
> > > > > Diffs
> > > > > -----
> > > > >
> > > > >
> > > >
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> > > > PRE-CREATION
> > > > >
> > > >
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> > > > 1330602
> > > > >
> > > >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> > > > 1330602
> > > > >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> > > > 1330602
> > > > >
> > > > > Diff: https://reviews.apache.org/r/4880/diff
> > > > >
> > > > >
> > > > > Testing
> > > > > -------
> > > > >
> > > > > Ran portal and witnessed no problems.
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Chris
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Jasha Joachimsthal <j....@onehippo.com>.
No idea who made the navigation in the admin interface, but it looks like a
great short term solution ;)
To be serious: there is already a proposal to split up the way pages are
built up [1]. It won't be available this release or the next, but in a few
months' time, pages will be built up in a modular way.

[1] http://markmail.org/thread/npuldma3yxibourj

Regards,

Jasha

On 26 April 2012 23:27, Chris Geer <ch...@cxtsoftware.com> wrote:

> Stan,
>
> I agree with you. I think I have a short term and a long term solution. The
> short term solution is to reuse the same concept that is used for the admin
> page right now where the model is given NavigationItems during page
> creation. Right now those are hardcoded in the controller. I'm going to
> replicate that for page, store and whatnot. I'm also going to add another
> task that will be to load those dynamically from the DB. That way plugins
> could add entries for each page type to the DB to get new nav items. And if
> a user wants to override the navbar they can just overwrite header.tag
> (which I plan on renaming since it's confusing with include/header.jsp).
>
> Chris
>
> On Thu, Apr 26, 2012 at 2:14 PM, Drozdetski, Stan A.
> <dr...@mitre.org>wrote:
>
> > I like the spirit of your second suggestion - just not sure about the
> > implementation.
> >
> > (I don't mean "not sure because I don't like it" - as a front-end guy, I
> > just don't know the right way to implement what you're asking for. But
> > getting this right is surprisingly critical for extensibility.)
> >
> > In the past, we've discussed the ability to extend Rave using
> > plugins/extensions/whatever. It would be mightily useful if a plugin is
> > able to dynamically add an item to a specified menu. If DB is the place
> to
> > do it, I'm all for it.
> >
> > The alternative - file replacement - is limiting when it comes to
> > customizing your instance, and then trying to upgrade to a new version of
> > Rave.
> >
> > (Background - the platform that I'm familiar with, Elgg, first allowed
> > plugins a programmatic way to append menu items. That quickly became hard
> > to manage, since some plugins wanted to override certain options added by
> > other plugins. In one of the recent releases, the menus changed - now, a
> > plugin can register and unregister menu items. You also have a provision
> to
> > specify the sort order for menu items. Saved us a lot of pain that would
> > otherwise be caused by file replacement.)
> >
> > Stan Drozdetski
> > MITRE
> >
> > -----Original Message-----
> > From: Chris Geer [mailto:chris@cxtsoftware.com]
> > Sent: Thursday, April 26, 2012 4:19 PM
> > To: dev@rave.apache.org
> > Subject: Re: Review Request: Separates the menubar from the page.jsp into
> > a standalone jsp for easy customization
> >
> > In regards to this I've got a couple of ideas (limited to navbar for the
> > moment):
> >
> > 1) The simplest approach is to define multiple navbar.tag files (i.e.
> > userpage_navbar.tag, store_navbar.tag) and that way each jsp file can
> > properly incorporate the correct navbar layout they want. Same thing
> could
> > be done with tiles.
> > 2) Another approach, like you suggested, is make them dynamic based on
> some
> > configuration. I like this approach but I'm not sure how we would drive
> it
> > out of the DB. Page_template would be the perfect place but most pages
> > aren't based on page templates at the moment. In fact, store.jsp doesn't
> > look like it pulls any data from the DB. I guess we could build it where
> it
> > was based on page_type and just have static values from some page types.
> So
> > maybe, STORE, USER, ADMIN, PERSON_PROFILE?
> >
> > If it was DB driven I envision a navitems table with the following
> columns:
> > id, page_type, menu_name, menu_item_name, menu_item_url, menu_item_icon.
> > (being to have multiple groups is important to us since we want to have
> the
> > items split between two dropdown menus)
> >
> > Parent User Story: https://issues.apache.org/jira/browse/RAVE-585
> >
> > Chris
> >
> > On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:
> >
> > >
> > > -----------------------------------------------------------
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviews.apache.org/r/4880/#review7265
> > > -----------------------------------------------------------
> > >
> > >
> > > Thanks for the patch.  A couple of issues though:
> > >
> > > 1) This only takes into account the portal pages.  Menus on other pages
> > > are rendered differently.  What would be the best approach is to have a
> > > single menubar with navigation contributions based on page template and
> > > render out the nav elements dynamically. This approach can then be
> > extended
> > > to allow menu item contributions from other sources.
> > >
> > > 2) This needs a JIRA ticket for us to Apply it :)
> > >
> > > - mfranklin
> > >
> > >
> > > On 2012-04-25 23:47:10, Chris Geer wrote:
> > > >
> > > > -----------------------------------------------------------
> > > > This is an automatically generated e-mail. To reply, visit:
> > > > https://reviews.apache.org/r/4880/
> > > > -----------------------------------------------------------
> > > >
> > > > (Updated 2012-04-25 23:47:10)
> > > >
> > > >
> > > > Review request for rave.
> > > >
> > > >
> > > > Summary
> > > > -------
> > > >
> > > > I pulled the "menubar" code out of page.jsp and put it into
> > > common/menubar.jsp to support easier customization for users. I added
> an
> > > optional attribute to the base-layout to support this additional field
> > and
> > > added it to the tile-def for standard pages.
> > > >
> > > > Something similar might be done for profile and store in the future
> but
> > > they can't use this exact JSP since their menu options differ from
> normal
> > > pages.
> > > >
> > > >
> > > > Diffs
> > > > -----
> > > >
> > > >
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> > > PRE-CREATION
> > > >
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> > > 1330602
> > > >
> > > /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> > > 1330602
> > > >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> > > 1330602
> > > >
> > > > Diff: https://reviews.apache.org/r/4880/diff
> > > >
> > > >
> > > > Testing
> > > > -------
> > > >
> > > > Ran portal and witnessed no problems.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Chris
> > > >
> > > >
> > >
> > >
> >
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
Stan,

I agree with you. I think I have a short term and a long term solution. The
short term solution is to reuse the same concept that is used for the admin
page right now where the model is given NavigationItems during page
creation. Right now those are hardcoded in the controller. I'm going to
replicate that for page, store and whatnot. I'm also going to add another
task that will be to load those dynamically from the DB. That way plugins
could add entries for each page type to the DB to get new nav items. And if
a user wants to override the navbar they can just overwrite header.tag
(which I plan on renaming since it's confusing with include/header.jsp).

Chris

On Thu, Apr 26, 2012 at 2:14 PM, Drozdetski, Stan A.
<dr...@mitre.org>wrote:

> I like the spirit of your second suggestion - just not sure about the
> implementation.
>
> (I don't mean "not sure because I don't like it" - as a front-end guy, I
> just don't know the right way to implement what you're asking for. But
> getting this right is surprisingly critical for extensibility.)
>
> In the past, we've discussed the ability to extend Rave using
> plugins/extensions/whatever. It would be mightily useful if a plugin is
> able to dynamically add an item to a specified menu. If DB is the place to
> do it, I'm all for it.
>
> The alternative - file replacement - is limiting when it comes to
> customizing your instance, and then trying to upgrade to a new version of
> Rave.
>
> (Background - the platform that I'm familiar with, Elgg, first allowed
> plugins a programmatic way to append menu items. That quickly became hard
> to manage, since some plugins wanted to override certain options added by
> other plugins. In one of the recent releases, the menus changed - now, a
> plugin can register and unregister menu items. You also have a provision to
> specify the sort order for menu items. Saved us a lot of pain that would
> otherwise be caused by file replacement.)
>
> Stan Drozdetski
> MITRE
>
> -----Original Message-----
> From: Chris Geer [mailto:chris@cxtsoftware.com]
> Sent: Thursday, April 26, 2012 4:19 PM
> To: dev@rave.apache.org
> Subject: Re: Review Request: Separates the menubar from the page.jsp into
> a standalone jsp for easy customization
>
> In regards to this I've got a couple of ideas (limited to navbar for the
> moment):
>
> 1) The simplest approach is to define multiple navbar.tag files (i.e.
> userpage_navbar.tag, store_navbar.tag) and that way each jsp file can
> properly incorporate the correct navbar layout they want. Same thing could
> be done with tiles.
> 2) Another approach, like you suggested, is make them dynamic based on some
> configuration. I like this approach but I'm not sure how we would drive it
> out of the DB. Page_template would be the perfect place but most pages
> aren't based on page templates at the moment. In fact, store.jsp doesn't
> look like it pulls any data from the DB. I guess we could build it where it
> was based on page_type and just have static values from some page types. So
> maybe, STORE, USER, ADMIN, PERSON_PROFILE?
>
> If it was DB driven I envision a navitems table with the following columns:
> id, page_type, menu_name, menu_item_name, menu_item_url, menu_item_icon.
> (being to have multiple groups is important to us since we want to have the
> items split between two dropdown menus)
>
> Parent User Story: https://issues.apache.org/jira/browse/RAVE-585
>
> Chris
>
> On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:
>
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/#review7265
> > -----------------------------------------------------------
> >
> >
> > Thanks for the patch.  A couple of issues though:
> >
> > 1) This only takes into account the portal pages.  Menus on other pages
> > are rendered differently.  What would be the best approach is to have a
> > single menubar with navigation contributions based on page template and
> > render out the nav elements dynamically. This approach can then be
> extended
> > to allow menu item contributions from other sources.
> >
> > 2) This needs a JIRA ticket for us to Apply it :)
> >
> > - mfranklin
> >
> >
> > On 2012-04-25 23:47:10, Chris Geer wrote:
> > >
> > > -----------------------------------------------------------
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviews.apache.org/r/4880/
> > > -----------------------------------------------------------
> > >
> > > (Updated 2012-04-25 23:47:10)
> > >
> > >
> > > Review request for rave.
> > >
> > >
> > > Summary
> > > -------
> > >
> > > I pulled the "menubar" code out of page.jsp and put it into
> > common/menubar.jsp to support easier customization for users. I added an
> > optional attribute to the base-layout to support this additional field
> and
> > added it to the tile-def for standard pages.
> > >
> > > Something similar might be done for profile and store in the future but
> > they can't use this exact JSP since their menu options differ from normal
> > pages.
> > >
> > >
> > > Diffs
> > > -----
> > >
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> > PRE-CREATION
> > >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> > 1330602
> > >
> > /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> > 1330602
> > >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> > 1330602
> > >
> > > Diff: https://reviews.apache.org/r/4880/diff
> > >
> > >
> > > Testing
> > > -------
> > >
> > > Ran portal and witnessed no problems.
> > >
> > >
> > > Thanks,
> > >
> > > Chris
> > >
> > >
> >
> >
>

RE: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by "Drozdetski, Stan A." <dr...@mitre.org>.
I like the spirit of your second suggestion - just not sure about the implementation.

(I don't mean "not sure because I don't like it" - as a front-end guy, I just don't know the right way to implement what you're asking for. But getting this right is surprisingly critical for extensibility.)

In the past, we've discussed the ability to extend Rave using plugins/extensions/whatever. It would be mightily useful if a plugin is able to dynamically add an item to a specified menu. If DB is the place to do it, I'm all for it.

The alternative - file replacement - is limiting when it comes to customizing your instance, and then trying to upgrade to a new version of Rave.

(Background - the platform that I'm familiar with, Elgg, first allowed plugins a programmatic way to append menu items. That quickly became hard to manage, since some plugins wanted to override certain options added by other plugins. In one of the recent releases, the menus changed - now, a plugin can register and unregister menu items. You also have a provision to specify the sort order for menu items. Saved us a lot of pain that would otherwise be caused by file replacement.)

Stan Drozdetski
MITRE

-----Original Message-----
From: Chris Geer [mailto:chris@cxtsoftware.com] 
Sent: Thursday, April 26, 2012 4:19 PM
To: dev@rave.apache.org
Subject: Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

In regards to this I've got a couple of ideas (limited to navbar for the
moment):

1) The simplest approach is to define multiple navbar.tag files (i.e.
userpage_navbar.tag, store_navbar.tag) and that way each jsp file can
properly incorporate the correct navbar layout they want. Same thing could
be done with tiles.
2) Another approach, like you suggested, is make them dynamic based on some
configuration. I like this approach but I'm not sure how we would drive it
out of the DB. Page_template would be the perfect place but most pages
aren't based on page templates at the moment. In fact, store.jsp doesn't
look like it pulls any data from the DB. I guess we could build it where it
was based on page_type and just have static values from some page types. So
maybe, STORE, USER, ADMIN, PERSON_PROFILE?

If it was DB driven I envision a navitems table with the following columns:
id, page_type, menu_name, menu_item_name, menu_item_url, menu_item_icon.
(being to have multiple groups is important to us since we want to have the
items split between two dropdown menus)

Parent User Story: https://issues.apache.org/jira/browse/RAVE-585

Chris

On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/#review7265
> -----------------------------------------------------------
>
>
> Thanks for the patch.  A couple of issues though:
>
> 1) This only takes into account the portal pages.  Menus on other pages
> are rendered differently.  What would be the best approach is to have a
> single menubar with navigation contributions based on page template and
> render out the nav elements dynamically. This approach can then be extended
> to allow menu item contributions from other sources.
>
> 2) This needs a JIRA ticket for us to Apply it :)
>
> - mfranklin
>
>
> On 2012-04-25 23:47:10, Chris Geer wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/
> > -----------------------------------------------------------
> >
> > (Updated 2012-04-25 23:47:10)
> >
> >
> > Review request for rave.
> >
> >
> > Summary
> > -------
> >
> > I pulled the "menubar" code out of page.jsp and put it into
> common/menubar.jsp to support easier customization for users. I added an
> optional attribute to the base-layout to support this additional field and
> added it to the tile-def for standard pages.
> >
> > Something similar might be done for profile and store in the future but
> they can't use this exact JSP since their menu options differ from normal
> pages.
> >
> >
> > Diffs
> > -----
> >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> PRE-CREATION
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> 1330602
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> 1330602
> >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> 1330602
> >
> > Diff: https://reviews.apache.org/r/4880/diff
> >
> >
> > Testing
> > -------
> >
> > Ran portal and witnessed no problems.
> >
> >
> > Thanks,
> >
> > Chris
> >
> >
>
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
In regards to this I've got a couple of ideas (limited to navbar for the
moment):

1) The simplest approach is to define multiple navbar.tag files (i.e.
userpage_navbar.tag, store_navbar.tag) and that way each jsp file can
properly incorporate the correct navbar layout they want. Same thing could
be done with tiles.
2) Another approach, like you suggested, is make them dynamic based on some
configuration. I like this approach but I'm not sure how we would drive it
out of the DB. Page_template would be the perfect place but most pages
aren't based on page templates at the moment. In fact, store.jsp doesn't
look like it pulls any data from the DB. I guess we could build it where it
was based on page_type and just have static values from some page types. So
maybe, STORE, USER, ADMIN, PERSON_PROFILE?

If it was DB driven I envision a navitems table with the following columns:
id, page_type, menu_name, menu_item_name, menu_item_url, menu_item_icon.
(being to have multiple groups is important to us since we want to have the
items split between two dropdown menus)

Parent User Story: https://issues.apache.org/jira/browse/RAVE-585

Chris

On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/#review7265
> -----------------------------------------------------------
>
>
> Thanks for the patch.  A couple of issues though:
>
> 1) This only takes into account the portal pages.  Menus on other pages
> are rendered differently.  What would be the best approach is to have a
> single menubar with navigation contributions based on page template and
> render out the nav elements dynamically. This approach can then be extended
> to allow menu item contributions from other sources.
>
> 2) This needs a JIRA ticket for us to Apply it :)
>
> - mfranklin
>
>
> On 2012-04-25 23:47:10, Chris Geer wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/
> > -----------------------------------------------------------
> >
> > (Updated 2012-04-25 23:47:10)
> >
> >
> > Review request for rave.
> >
> >
> > Summary
> > -------
> >
> > I pulled the "menubar" code out of page.jsp and put it into
> common/menubar.jsp to support easier customization for users. I added an
> optional attribute to the base-layout to support this additional field and
> added it to the tile-def for standard pages.
> >
> > Something similar might be done for profile and store in the future but
> they can't use this exact JSP since their menu options differ from normal
> pages.
> >
> >
> > Diffs
> > -----
> >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> PRE-CREATION
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> 1330602
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> 1330602
> >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> 1330602
> >
> > Diff: https://reviews.apache.org/r/4880/diff
> >
> >
> > Testing
> > -------
> >
> > Ran portal and witnessed no problems.
> >
> >
> > Thanks,
> >
> > Chris
> >
> >
>
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
Matt,

Ya, I thought I cancelled the review. I realized it wasn't good enough. I
did just create a bunch on JIRA tickets and I'll resubmit a patch later
today.

Chris

On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/#review7265
> -----------------------------------------------------------
>
>
> Thanks for the patch.  A couple of issues though:
>
> 1) This only takes into account the portal pages.  Menus on other pages
> are rendered differently.  What would be the best approach is to have a
> single menubar with navigation contributions based on page template and
> render out the nav elements dynamically. This approach can then be extended
> to allow menu item contributions from other sources.
>
> 2) This needs a JIRA ticket for us to Apply it :)
>
> - mfranklin
>
>
> On 2012-04-25 23:47:10, Chris Geer wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/
> > -----------------------------------------------------------
> >
> > (Updated 2012-04-25 23:47:10)
> >
> >
> > Review request for rave.
> >
> >
> > Summary
> > -------
> >
> > I pulled the "menubar" code out of page.jsp and put it into
> common/menubar.jsp to support easier customization for users. I added an
> optional attribute to the base-layout to support this additional field and
> added it to the tile-def for standard pages.
> >
> > Something similar might be done for profile and store in the future but
> they can't use this exact JSP since their menu options differ from normal
> pages.
> >
> >
> > Diffs
> > -----
> >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> PRE-CREATION
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> 1330602
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> 1330602
> >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> 1330602
> >
> > Diff: https://reviews.apache.org/r/4880/diff
> >
> >
> > Testing
> > -------
> >
> > Ran portal and witnessed no problems.
> >
> >
> > Thanks,
> >
> > Chris
> >
> >
>
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
Matt,

I pulled the rename from the patch and created a new ticket to make that
happen (post the patch). Patch is up for review again.

Chris

On Sun, Apr 29, 2012 at 1:28 PM, Franklin, Matthew B.
<mf...@mitre.org>wrote:

> A patch is probably a difficult way to do a rename of a file.  Review
> board might be having trouble figuring out what relates to what.
>
> I don't have any issue with the rename.  Why don't you roll it back for
> the patch and just request in the ticket that it gets renamed to navbar.tag.
>
> -Matt
>
> From: Chris Geer [mailto:chris@cxtsoftware.com]
> Sent: Friday, April 27, 2012 1:18 PM
> To: dev@rave.apache.org
> Subject: Re: Review Request: Separates the menubar from the page.jsp into
> a standalone jsp for easy customization
>
> I've made the changes I wanted and now I'm trying to resubmit the review
> but having a problem. One of the changes I made was I renamed the
> header.tag to navbar.tag since it better described what it really was.
> However, when I try and submit the review I get the following error from my
> patch file.
>
> The file
> '/trunk/rave-portal-resources/src/main/webapp/WEB-INF/tags/navbar.tag'
> (r1331497) could not be found in the repository
>
> Any thoughts? Attached is the patch.
>
> Chris
>
> On Thu, Apr 26, 2012 at 8:25 AM, <mfranklin@apache.org<mailto:
> mfranklin@apache.org>> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/#review7265
> -----------------------------------------------------------
>
>
> Thanks for the patch.  A couple of issues though:
>
> 1) This only takes into account the portal pages.  Menus on other pages
> are rendered differently.  What would be the best approach is to have a
> single menubar with navigation contributions based on page template and
> render out the nav elements dynamically. This approach can then be extended
> to allow menu item contributions from other sources.
>
> 2) This needs a JIRA ticket for us to Apply it :)
>
> - mfranklin
>
>
> On 2012-04-25 23:47:10, Chris Geer wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/
> > -----------------------------------------------------------
> >
> > (Updated 2012-04-25 23:47:10)
> >
> >
> > Review request for rave.
> >
> >
> > Summary
> > -------
> >
> > I pulled the "menubar" code out of page.jsp and put it into
> common/menubar.jsp to support easier customization for users. I added an
> optional attribute to the base-layout to support this additional field and
> added it to the tile-def for standard pages.
> >
> > Something similar might be done for profile and store in the future but
> they can't use this exact JSP since their menu options differ from normal
> pages.
> >
> >
> > Diffs
> > -----
> >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> PRE-CREATION
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> 1330602
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> 1330602
> >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> 1330602
> >
> > Diff: https://reviews.apache.org/r/4880/diff
> >
> >
> > Testing
> > -------
> >
> > Ran portal and witnessed no problems.
> >
> >
> > Thanks,
> >
> > Chris
> >
> >
>
>

RE: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by "Franklin, Matthew B." <mf...@mitre.org>.
A patch is probably a difficult way to do a rename of a file.  Review board might be having trouble figuring out what relates to what.

I don't have any issue with the rename.  Why don't you roll it back for the patch and just request in the ticket that it gets renamed to navbar.tag.

-Matt

From: Chris Geer [mailto:chris@cxtsoftware.com]
Sent: Friday, April 27, 2012 1:18 PM
To: dev@rave.apache.org
Subject: Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

I've made the changes I wanted and now I'm trying to resubmit the review but having a problem. One of the changes I made was I renamed the header.tag to navbar.tag since it better described what it really was. However, when I try and submit the review I get the following error from my patch file.

The file '/trunk/rave-portal-resources/src/main/webapp/WEB-INF/tags/navbar.tag' (r1331497) could not be found in the repository

Any thoughts? Attached is the patch.

Chris

On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org>> wrote:

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4880/#review7265
-----------------------------------------------------------


Thanks for the patch.  A couple of issues though:

1) This only takes into account the portal pages.  Menus on other pages are rendered differently.  What would be the best approach is to have a single menubar with navigation contributions based on page template and render out the nav elements dynamically. This approach can then be extended to allow menu item contributions from other sources.

2) This needs a JIRA ticket for us to Apply it :)

- mfranklin


On 2012-04-25 23:47:10, Chris Geer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/
> -----------------------------------------------------------
>
> (Updated 2012-04-25 23:47:10)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> I pulled the "menubar" code out of page.jsp and put it into common/menubar.jsp to support easier customization for users. I added an optional attribute to the base-layout to support this additional field and added it to the tile-def for standard pages.
>
> Something similar might be done for profile and store in the future but they can't use this exact JSP since their menu options differ from normal pages.
>
>
> Diffs
> -----
>
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp PRE-CREATION
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1330602
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330602
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml 1330602
>
> Diff: https://reviews.apache.org/r/4880/diff
>
>
> Testing
> -------
>
> Ran portal and witnessed no problems.
>
>
> Thanks,
>
> Chris
>
>


Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by Chris Geer <ch...@cxtsoftware.com>.
I've made the changes I wanted and now I'm trying to resubmit the review
but having a problem. One of the changes I made was I renamed the
header.tag to navbar.tag since it better described what it really was.
However, when I try and submit the review I get the following error from my
patch file.

The file
'/trunk/rave-portal-resources/src/main/webapp/WEB-INF/tags/navbar.tag'
(r1331497) could not be found in the repository

Any thoughts? Attached is the patch.

Chris

On Thu, Apr 26, 2012 at 8:25 AM, <mf...@apache.org> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/#review7265
> -----------------------------------------------------------
>
>
> Thanks for the patch.  A couple of issues though:
>
> 1) This only takes into account the portal pages.  Menus on other pages
> are rendered differently.  What would be the best approach is to have a
> single menubar with navigation contributions based on page template and
> render out the nav elements dynamically. This approach can then be extended
> to allow menu item contributions from other sources.
>
> 2) This needs a JIRA ticket for us to Apply it :)
>
> - mfranklin
>
>
> On 2012-04-25 23:47:10, Chris Geer wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/4880/
> > -----------------------------------------------------------
> >
> > (Updated 2012-04-25 23:47:10)
> >
> >
> > Review request for rave.
> >
> >
> > Summary
> > -------
> >
> > I pulled the "menubar" code out of page.jsp and put it into
> common/menubar.jsp to support easier customization for users. I added an
> optional attribute to the base-layout to support this additional field and
> added it to the tile-def for standard pages.
> >
> > Something similar might be done for profile and store in the future but
> they can't use this exact JSP since their menu options differ from normal
> pages.
> >
> >
> > Diffs
> > -----
> >
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp
> PRE-CREATION
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
> 1330602
> >
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp
> 1330602
> >   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml
> 1330602
> >
> > Diff: https://reviews.apache.org/r/4880/diff
> >
> >
> > Testing
> > -------
> >
> > Ran portal and witnessed no problems.
> >
> >
> > Thanks,
> >
> > Chris
> >
> >
>
>

Re: Review Request: Separates the menubar from the page.jsp into a standalone jsp for easy customization

Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4880/#review7265
-----------------------------------------------------------


Thanks for the patch.  A couple of issues though:

1) This only takes into account the portal pages.  Menus on other pages are rendered differently.  What would be the best approach is to have a single menubar with navigation contributions based on page template and render out the nav elements dynamically. This approach can then be extended to allow menu item contributions from other sources.

2) This needs a JIRA ticket for us to Apply it :)

- mfranklin


On 2012-04-25 23:47:10, Chris Geer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4880/
> -----------------------------------------------------------
> 
> (Updated 2012-04-25 23:47:10)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> I pulled the "menubar" code out of page.jsp and put it into common/menubar.jsp to support easier customization for users. I added an optional attribute to the base-layout to support this additional field and added it to the tile-def for standard pages.
> 
> Something similar might be done for profile and store in the future but they can't use this exact JSP since their menu options differ from normal pages.
> 
> 
> Diffs
> -----
> 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/common/menubar.jsp PRE-CREATION 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1330602 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330602 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/tiles-defs.xml 1330602 
> 
> Diff: https://reviews.apache.org/r/4880/diff
> 
> 
> Testing
> -------
> 
> Ran portal and witnessed no problems.
> 
> 
> Thanks,
> 
> Chris
> 
>