TheCoder4eu / BootsFacesWeb

BootsFaces : the next gen JSF Framework Web Docs
Apache License 2.0
36 stars 32 forks source link

[WIP] Consistency fixes #36

Closed ggam closed 7 years ago

ggam commented 7 years ago

KNOWN ISSUES:

Temporary skipped pages:

This is a WIP providing a unified look for all the examples.

This doesn't change example contents, just visual changes, based on the documentation of the most recent components.

I've done the Miscelaneous section and the first 7th pages of he Layout section. I'd like to have it finished tomorrow at most so it can get into 1.1.0.

How do you see it?

1 2

ggam commented 7 years ago

Btw, this is a work in progress since I haven't touched all the pages. Current work can be merged as is anyway.

ggam commented 7 years ago

horizontalForms page is conflicting but I think you have more changes to do on that page so I will wait to resolve it. The other changes could be cherrypicked.

stephanrauh commented 7 years ago

I'm trying to figure out what you're trying to do. An important part seems to be to put the source code below the live demos. Is that correct?

I'm not entirely sure because it makes the long pages even longer, but it might be an improvement to see everything at a glance.

While you're at it: recently I've stumbled over demo with the Java source code missing. Most of the time it's not important, but there were some examples that were hard to understand without the Java code. Maybe you can have an eye on completeness.

stephanrauh commented 7 years ago

Sorry for the horizontal form - but it should be easy to fix.

stephanrauh commented 7 years ago

Other hints for consistency (not urgent, just wanted to mention them):

ggam commented 7 years ago

Sorry for not explaininh before.

Yes, I'm just mainly moving the source below the demos to get a consistent experience.

As it was until now, there were some examples already with this format, others with tabs showing the source tab by default, others lacked needed source (fullCalendar), etc.

As a user, I can tell you it is not confortable and specially inconsistent. Apart from it, there are some examples using deprecated attributes or using h:form where b:form would be a better suit.

Viewing the demo and the source at the same time (as it was being done in most recent components) makes it easy to see what's going on and will benefit from a "copy to clipboard" button.

Yes, some pages will grow, but having a unified look will allos us to improve it as a whole. A sidebar would help there. But that's something to look at later.

And yes, in this process I plan to review and add the missing java bean sources where needed ;). For now I'm just on visual things so we also reduce the risk of conflicts.

El vie., 21 de abril de 2017 20:40, Stephan Rauh notifications@github.com escribió:

I'm trying to figure out what you're trying to do. An important part seems to be to put the source code below the live demos. Is that correct?

I'm not entirely sure because it makes the long pages even longer, but it might be an improvement to see everything at a glance.

While you're at it: recently I've stumbled over demo with the Java source code missing. Most of the time it's not important, but there were some examples that were hard to understand without the Java code. Maybe you can have an eye on completeness.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/TheCoder4eu/BootsFacesWeb/pull/36#issuecomment-296270527, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAucGO7l1X5h3gdA8QpEV60-WMyMj7Wks5ryPgWgaJpZM4NEjcN .

stephanrauh commented 7 years ago

Another hint for consistency: if you're using Netbeans, run the source code formatter. Eclipse formats XHTML file so badly they are illegible after formatting. Netbeans seems to do it better. I didn't check IntelliJ yet. If possible, run the source code formatting in a separate commit, so the history can be tracked easily.

stephanrauh commented 7 years ago

BTW, I dream of a component taking the source code as a parameter, rendering the live demo and showing the source code. This seems to be impossible because JSF parses the XML file, so there's no trace of the original source code, but still, it would be useful to have such a component.

ggam commented 7 years ago

I've now finished the layout section. Please take a look at https://github.com/TheCoder4eu/BootsFacesWeb/pull/36/files#diff-600376dffeb79835ede4a0b285078036L252

It was putting the fontAwesomeList.txt into WEB-INF/classes/WEB-INF/classes directory. Also I refactored IconAwesomeBean into a more robust solution: https://github.com/TheCoder4eu/BootsFacesWeb/pull/36/files#diff-ff2e218b0ffa60f25a9f334001d1439dR35

There are two conflicts now but they should be easy to fix. I will take a look tomorrow.

As for your other comments, I'll create a new issue to track them all, and I'll format code once I finished.

ggam commented 7 years ago

@stephanrauh I'm adding found bugs to the description of this pull request as I encounter them. I don't think the two I've already struggled with are coming my changes, but I cannot access http://www3.bootsfaces.net/Showcase (it redirects me to http://staging01:28080/Showcase/). Could you please check if they are working now on the snapshot showcase?

For the default command problem, the first click gives the following response (note the PF script there) and nothing is updated:

<?xml version='1.0' encoding='UTF-8'?>
<partial-response id="j_id1"><changes><eval><![CDATA[PrimeFaces.ajax.Utils.loadStylesheets(['/javax.faces.resource/css/default/core.css.jsf?ln=bsf','/javax.faces.resource/css/default/theme.css.jsf?ln=bsf','/javax.faces.resource/css/scrollup.css.jsf?ln=bsf','/javax.faces.resource/css/dropdown-submenu.css.jsf?ln=bsf','/javax.faces.resource/css/bootstrap-treeview.min.css.jsf?ln=bsf','/javax.faces.resource/css/bsf.css.jsf?ln=bsf']);PrimeFaces.ajax.Utils.loadScripts(['/javax.faces.resource/jq/jquery.js.jsf?ln=bsf','/javax.faces.resource/js/jquery.scrollUp.min.js.jsf?ln=bsf','/javax.faces.resource/jsf.js.jsf?ln=javax.faces','/javax.faces.resource/js/alert.js.jsf?ln=bsf','/javax.faces.resource/js/tooltip.js.jsf?ln=bsf','/javax.faces.resource/js/tab.js.jsf?ln=bsf','/javax.faces.resource/js/bsf.js.jsf?ln=bsf']);]]></eval><update id="j_idt177:standardMsg"><![CDATA[<div class="bf-messages  " role="alert" id="j_idt177:standardMsg"><div class="alert fadein alert-info" style="padding:15px;margin-top:10px;"><a class="close" data-dismiss="alert" href="#">&times;</a><span class="bf-message"><span class="bficon bficon-info bf-message-icon" aria-hidden="true"></span><strong><span class="bf-message-summary">BUTTON 3</span></strong><span class="bf-message-detail">BUTTON 3 WAS PRESSED</span></span></div></div>]]></update><update id="j_id1:javax.faces.ViewState:0"><![CDATA[4112084560593001631:-7673190528759267161]]></update></changes></partial-response>

Clicking a second time gives me the following response, showing the message:

<?xml version='1.0' encoding='UTF-8'?>
<partial-response id="j_id1"><changes><update id="j_idt177:standardMsg"><![CDATA[<div class="bf-messages  " role="alert" id="j_idt177:standardMsg"><div class="alert fadein alert-info" style="padding:15px;margin-top:10px;"><a class="close" data-dismiss="alert" href="#">&times;</a><span class="bf-message"><span class="bficon bficon-info bf-message-icon" aria-hidden="true"></span><strong><span class="bf-message-summary">BUTTON 3</span></strong><span class="bf-message-detail">BUTTON 3 WAS PRESSED</span></span></div></div>]]></update><update id="j_id1:javax.faces.ViewState:0"><![CDATA[4112084560593001631:-7673190528759267161]]></update></changes></partial-response>
stephanrauh commented 7 years ago

http://www3.bootsfaces.net/Showcase/forms/DefaultCommand.jsf still works, and the buttons work as expected.

stephanrauh commented 7 years ago

I can reproduce the bug on my local machine.

ggam commented 7 years ago

You reproduce it with master or with my changes? I'm developing with Maven build and Tomcat (showcase didn't work on Payara, probably because of the build profile)

stephanrauh commented 7 years ago

Both master and your branch.

ggam commented 7 years ago

So maybe they are environment related... I'm know facing a problem with the Datatable with custom options and the JavaScript code for the "i18nbcarPool2Table" not being rendered.

Production environment is WildFly with Gradle build? Any other instructions to replicate it locally so I can see if the problem is not mine?

stephanrauh commented 7 years ago

It seems to be a problem of PrimeFaces 6.0. Updating to 6.1 fixes the bug. So does downgrading to 5.2.

@tandraschko Does that ring a bell? Something like PrimeFaces.ajax.Utils.loadStylesheets() fails because window.PrimeFaces is not defined, and that in turn causes the entire AJAX update to fail?

ggam commented 7 years ago

And that's the reason why I proposed to separate third party libraries into different wars :)

Could PrimeFaces be upgraded to 6.1? Or is it too risky at this moment?

tandraschko commented 7 years ago

@stephanrauh not sure but may be a bug. PrimeFaces dynamic resource loading is always active, doesn't matter if f:ajax or p:ajax is used. But if you don't have a PF component in the view, the script is not loaded and window.PrimeFaces is ofc undefined.

UPDATE: fixed exactly that bug in 6.1 ;)

stephanrauh commented 7 years ago

@ggam Did I forget to mention that? Yes, I've updated PrimeFaces to 6.1.

ggam commented 7 years ago

@tandraschko Thanks for the info!

@stephanrauh Perfect! I'll try now. I'm finishing the InputText page. As a whole this is being a lot more work than I originally expected, as I'm already fixing outdated examples and such. But that's also the main reason I'd like to get this into 1.1.0.

In order to prevent more conflicts and to be able to test it ASAP, could this be merged as is when I finish the inputText page and fixed the horizontalForms page again? Or will this need to wait post 1.1.0?

ggam commented 7 years ago

@stephanrauh I've just finished the inputText page. Please I'd like you to review the contents. I think some of them are incorrect.

Specially the section regarding inline forms. I haven't touched it cause I think it has to be totally refactored since responsive classes don't work on inline classes (or at least not that way, maybe using <b:row /> is ok).

inline

I also added a note on the responsive design section stating that it only applies to horizontal forms. That should be corroborated from someone.

The only known issue now is the one of the Datatable. I need some help there if you have some minutes. The JS for the widget is not being rendered and there are two images not being loaded.

stephanrauh commented 7 years ago

Oh dear! I wasn't aware that the showcase pages are that old. It's good someone cares about that!

I've tried to send you a pull request to your pull request, but instead I seem to have managed to update your branch on the main repository. Can you see my changes?

image

ggam commented 7 years ago

Theoretically, maintainers can push to the branch of the pull request (by default; the creator of the pull request can change that).

Feel free to push to my branch, will be easier this way.

But I don't see the changes neither on my branch nor in https://github.com/TheCoder4eu/BootsFacesWeb/pull/36/commits

I'm on mobile, I'll check tomorrow on the PC.

El sáb., 22 de abril de 2017 23:03, Stephan Rauh notifications@github.com escribió:

Oh dear! I wasn't aware that the showcase pages are that old. It's good someone cares about that!

I've tried to send you a pull request to your pull request, but instead I seem to have managed to update your branch on the main repository. Can you see my changes?

[image: image] https://cloud.githubusercontent.com/assets/3045767/25308195/db529d6a-27af-11e7-9807-7fde85582c33.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TheCoder4eu/BootsFacesWeb/pull/36#issuecomment-296401375, or mute the thread https://github.com/notifications/unsubscribe-auth/ACAucGl0ehYHM6_O9J33ViPKy6M3Xqw_ks5rymsTgaJpZM4NEjcN .

stephanrauh commented 7 years ago

I don't have any idea how this is possible, but my push seems to have arrived in the consistency branch of the main repository: https://github.com/TheCoder4eu/BootsFacesWeb/commit/8af333d45329fd9c13039ca31c255da99d36c2b5

ggam commented 7 years ago

That's what I saw yesterday hahaha. Anyway, I've just merged that branch with my own and your commit is now incorporated.

stephanrauh commented 7 years ago

Ah, that's why modifying the readme.md made yesterday's commit appear magically :). It seems I can push directly to your fork. I didn't expect that. Very useful!

stephanrauh commented 7 years ago

Yesterday, I added look="primary" to the panels. I'm a bit fed up with all these gray pages, so I thought adding color might make the page look more friendly. What do you think?

ggam commented 7 years ago

Some panels had look="info", which I think offers better semantics here. I still haven't added them as I plan to revisit all of this to replace them with a composite component with look="XX" collapsible="false". But that should be done after this PR is merged.

Could you please review the "Inline forms" section? I'm not sure if that makes sense, or if we should add <b:row />'s to the examples there.

stephanrauh commented 7 years ago

I think it looks better now. The inline section doesn't follow the new styleguide, but at least it doesn't show outdated or wrong code.

stephanrauh commented 7 years ago

I've fixed the datatable demos.

ggam commented 7 years ago

Thanks!

ggam commented 7 years ago

Btw, I'm changing examples involving JavaScript to use CSS classes instead of element ids to select elements, as it is the recommended way to work with JSF. That will make the examples more robust.

Could this be used on the datatable too?

ggam commented 7 years ago

@stephanrauh I've found that the ajax example of the datetimepicker doesn't work. I calls a method of a bean named "test", but there are two beans with the same name.

A warning appears when deploying the showcase, warning that there are more duplicated beans:


23-Apr-2017 17:30:26.555 ADVERTENCIA [http-nio-8080-exec-48] com.sun.faces.mgbean.BeanManager.addBean JSF1074: ya se ha registrado el bean administrado denominado 'carBean'.  Sustituyendo el tipo de clase de bean administrado de.beyondjava.jsf.sample.carshop.CarBean existente por net.bootsfaces.demo.CarBean.
23-Apr-2017 17:30:26.623 ADVERTENCIA [http-nio-8080-exec-48] com.sun.faces.mgbean.BeanManager.addBean JSF1074: ya se ha registrado el bean administrado denominado 'now'.  Sustituyendo el tipo de clase de bean administrado java.util.Date existente por java.util.Date.```

The error was already there before my changes. Could you please take a look at it?
ggam commented 7 years ago

I've push some more changes. Responsive labels doesn't seem to work correctly. Doesn't seem related to TheCoder4eu/BootsFaces-OSP#742

captura3

ggam commented 7 years ago

Thanks!

stephanrauh commented 7 years ago

Te agradezco también :).