Closed exabrial closed 3 years ago
Hey, cool, yesterday I've read your question on StackOverflow and wondered how to answer it, and today you've already got a solution. Color me impressed!
First impression: I'm pretty sure your solution works. I'm afraid I need to restore my BootsFaces development environment, so I couldn't run a test today. Please nudge me again if I haven't given you a definite answer next Monday.
Do you happen to know if the for
loop is really necessary? At the time, I've implemented it because I wasn't aware converters are such a popular thing in the JSF world. I wonder if the implicit converter is an implementation of my for
loop.
Best regards, Stephan
Oh, and before I forget: thanks for all the kind words! That's what keeps an open source developer going. That and the 3000 downloads each month. It's a pity there's so little activity in this project recently. Would you like to help us? I can't afford to devote as much time as I used to do to BootsFaces, but I'm sure I still can guide and coordinate developers who'd like to fill the gap. Anybody interested?
Do you happen to know if the for loop is really necessary?
Generally that's what converters are for. It's handy to have a fallback with the loop, but generally a converter is the recommended way in JSF. They also have a lot of uses from displaying version text of objects or for loading stuff on page render.
I still can guide and coordinate developers who'd like to fill the gap. Anybody interested?
:) We're interested. We use quite a bit of BootsFaces, and we'll submit patches for bugs that affect us. The big thing for us is while we're users of JSF frameworks, we don't know much about their implementation. That's why I'm not sure our patch is completely correct... we'd love to know if all of the conditions are satisfied.
We've got a deal! Only it's not a deal because this is open source country... and everything anybody's doing here is voluntary, a gift to the community. So let me try again: welcome to the team!
we're users of JSF frameworks, we don't know much about their implementation
Don't worry. There are only half a dozen sophisticated algorithms in BootsFaces. The vast majority of the code is simple and straight-forward. Mind you: you've already managed to dig into one of the hard parts.
we'd love to know if all of the conditions are satisfied.
If it works, it's good. :) We never managed to write automated tests (lots of attempts, all of them failures), so we compensate this with the showcase. Clone the showcase (https://github.com/TheCoder4eu/BootsFacesWeb), add a demo to https://github.com/TheCoder4eu/BootsFacesWeb/blob/master/src/main/webapp/forms/selectOneMenu.xhtml, and check if both the new feature and the old demos work. If they do, send us a pull request to the showcase and the library and nudge me until I've merged it. That's all!
Alright, awesome! We'll do some testing and turn a patch around by the weekend.
Can you comment on https://github.com/TheCoder4eu/BootsFaces-OSP/issues/1164 and tell us what you think the root issue is and how to go about investigating/fixing? We'll do the leg work from there, just need a bit of direction!
Done! Don't hesitate to tell me if I didn't manage to answer all your question. You've dug really deep into the source code, so I know you'll manage to solve the issue.
BTW, now you've found the most annoying piece of code (the AddResourcesListener
) and the second most convoluted class (the DataTableRenderer
). :)
@stephanrauh Patch submitted. Two things I'm unsure of:
1) The original code calls setLocalValue
at one point. I have no idea what that means/does. I took it out.
2) on line 74, I call menu.setSubmittedValue(convertedValue);
rather than menu.setSubmittedValue(submittedOptionValue);
. I believe this is supposed to be the correct course of action, and it does indeed work, but again, not an expert on JSF internals.
Thanks for submitting the PR and for pointing the two changes out. I'll try to remember why I called setLocalValue()
. It was one of the first components I ever wrote, so it's possible I made a lot of mistakes. But maybe there was a reason.
I've merged your PR and uploaded it to Maven Central as snapshot 1.6.0. See issue #369 on how to get it.
Thanks a lot! Stephan
Hey guys! First I just want to say thanks for Bootsfaces. Open Source work frequently goes unthanked, and I'd just like to extend my gratitude :)
So I think we found an issue with SelectOneMenuRenderer when using a converter. Essentially what I think we found is
getAsObject()
isn't called.Here's the stack trace, which is a bit misleading:
So what's happening in SelectOneMenuRenderer is it looks like it has a default way of converting menu items into objects (sort of like a default converter). This handy feature works by iterating through the SelectItems and trying to match one up with the submittedValue. See here: https://github.com/TheCoder4eu/BootsFaces-OSP/blob/master/src/main/java/net/bootsfaces/component/selectOneMenu/SelectOneMenuRenderer.java#L68
We have a temporary patch:
Ideally the code would probably use the converter if possible, and in leui of that, execute the for loop and try to match a value.
We're totally willing and able to write a patch if we can get some direction on what you want and your expectations. Please let us know how to proceed :)
THANK YOU!