i have updated my review after the recent changes:
--snip--
Second review after fixes (2009-02-02)
--------------------------------------
After fixes by Wichert and Danny the picture looks much prettier :-)
* All but two tests passed. One is explicitely related to another
ticket and irrelevant to this PLIP.
* The other simply failed due to the changes made by this PLIP, so I
took the liberty of `fixing it
on the spot<https://dev.plone.org/plone/changeset/24828>`_
* Manual tests worked fine without anything unexpected.
* I do think the icons look too different from the rest of Plone's
default look but that's just
my personal taste, no showstopper.
conclusion (updated)
--------------------
After the recent changes I am now +1 on merging.
--snap--
On 01.02.2009, at 13:33, Danny Bloemendaal wrote:
I added my review notes:
PLIP 243 framework UI review by Danny Bloemendaal (ender_), 2009-02-01
======================================================================
I reviewed this bundle and it all looks very familiair to me (ok,
the markup and ideas where taken from our plone intranet) ;-). Anyway
it all looks good and works as expected (the bugs mentioned below seem
to be fixed). I added some css and icons
to make it look good (imho).
One remark: I'd like to see a time stamp as well in the revision
dropdowns when viewing diffs. There can me more than one revision
during
the day and then the time information helps.
Conclusion
----------
+1 to integrate.
On 18 jan 2009, at 00:50, Wichert Akkerman wrote:
For reference here are the implementation notes. They are also
present in the
README.txt in the buildout itself.
Implementation notes
====================
This buildout contains the base implementation for PLIP 243:
replace the
standard workflow history viewlet with a content history viewlet.
This buildout contains two changes:
- plone.app.layout has a new content history viewlet. This viewlet
shows
both versioning and workflow changes, and provides direct options to
show the differences between versions, review an older version or
revert
to an older version.
- a new history view was added which provides a much simpler way to
show the differences between different revisions.
Needed documentation changes
----------------------------
User manuals should be updated to show the content viewlet and
describe the
actions it exposes.
Outstanding issues
------------------
- the html_diff code difference view can trigger a
UnicodeDecodeError, so I
disabled it for now. For some unknown reason this only happens in the
history browser view. The exact same diff works fine when shown via
the
version_diff CMF page template.
- the CSS and icons needed for the content history viewlet are still
missing. My CSS skills are too limited and my graphics skills
non-existant, so someone else will need to tackle this. I can show
screenshots of the viewlet as it is running on a customer site
if desirable.
- it might be desirable to add a migration step to Plone to remove
the
old history action.
- the permission used for the @@history form may need to be verified
--
Wichert Akkerman <wich...@wiggy.net> It is simple to make things.
http://www.wiggy.net/ It is hard to make things
simple.
_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
