Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add RIESelect component #10

Merged
merged 4 commits into from
Sep 26, 2016
Merged

add RIESelect component #10

merged 4 commits into from
Sep 26, 2016

Conversation

tim-phillips
Copy link
Contributor

I needed a select component in a project I'm working on and figured I'd do a solid and add it to this repo, I really like what you've build here.

Let me know if there's anything I could change in this commit. I chose to pass the select options as an object, but it could just as easily be passed as a list of objects like so:

[
  { value: "1", text: "one" },
  { value: "2", text: "two" },
  { value: "3", text: "three" },
  { value: "4", text: "four" }
]
  • could be clearer
  • more typing
  • can use map instead of for loop

I had to put a conditional in RIEBase.js since <select> elements don't have the function setSelectionRange.

@tim-phillips
Copy link
Contributor Author

tim-phillips commented Aug 24, 2016

I just commited 8f70f07, I think that passing select options as an object is the best way to go. It's more elegant and makes it easier to convert data from a database.

I changed value to id to avoid confusion because the word 'value' is already used throughout the code.

@tim-phillips
Copy link
Contributor Author

tim-phillips commented Aug 26, 2016

If we don't want to edit the base class then we could redefine selectInputText in RIESelect. Let me know if so and I'll change it.

@KyleAMathews
Copy link

Just installed this PR and played around with it some in a site I'm building. It works just fine but I question if it's necessary? With this in-place select component you have to click it twice to edit it whereas with a normal select you need to click just once which seems more in-line with this lib's philosophy.

Thoughts? Perhaps just recommend people use a normal <select>.

@tim-phillips
Copy link
Contributor Author

Thanks for checking out the PR!

I'd like to not see a select box when displaying the data but I'd like to see the select box when editing. Clicking twice is inconvenient, but figuring out how to open the select box on the first click would be a great future PR.

@KyleAMathews
Copy link

You can hide all the select-looking stuff with CSS so it just looks like text until you click it.

screen shot 2016-09-17 at 10 19 02 am

That's a screenshot from a current project. "purchase" is the select.

@kaivi kaivi merged commit 1c11536 into kaivi:master Sep 26, 2016
@kaivi
Copy link
Owner

kaivi commented Sep 26, 2016

Hey, sorry it took so long, It is merged and published as v 1.0.5 now!

@KyleAMathews
Copy link

To open the select immediately you can I believe just pass the click through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants