Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-955: Add database selection submit button in catalogview.jsp for text based browse.#80

Closed
babokim wants to merge 1 commit into
apache:masterfrom
babokim:TAJO-955
Closed

TAJO-955: Add database selection submit button in catalogview.jsp for text based browse.#80
babokim wants to merge 1 commit into
apache:masterfrom
babokim:TAJO-955

Conversation

@babokim
Copy link
Copy Markdown
Contributor

@babokim babokim commented Jul 17, 2014

No description provided.

@jihoonson
Copy link
Copy Markdown
Contributor

Hi Hyoungjun.
This issue makes sense for text-based browsers, but the submit button is useless in graphical browsers. How about enforce pushing the submit button to change the database selection?

@babokim
Copy link
Copy Markdown
Contributor Author

babokim commented Jul 18, 2014

Hi jihoonson.
I use a elinks browser and this browser does not support a onchange javascript event. This is why I added the button. Do you means removing onchange event from selection component?

@jihoonson
Copy link
Copy Markdown
Contributor

Right. In my opinion, it will give users the consistent experience regardless of the browser type.

@hyunsik
Copy link
Copy Markdown
Member

hyunsik commented Jul 20, 2014

In my opinion, the current approach using on-change event is also good. It is definitely convenient for users. Because only a few of users who use some web browsers which does not support java script will use submit button, we should kept on-change event for most of users.

@jihoonson
Copy link
Copy Markdown
Contributor

I don't think that we SHOULD keep on-change event. Why do we have an obligation of adding a button which will be useless for most users?
I think that it can be our choice, and the above comment was just my suggestion.
If you guys don't agree, the current patch is also good.

@jihoonson
Copy link
Copy Markdown
Contributor

I forgot +1.

@hyunsik
Copy link
Copy Markdown
Member

hyunsik commented Jul 20, 2014

Thank you for your understanding.

@babokim Could you close this issue?

@babokim babokim closed this Jul 21, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants