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.

Sign up for GitHub

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

Jump to bottom

allow multiline mode for input with type text (close #458) #1155

Closed
wants to merge 1 commit into from
Closed

allow multiline mode for input with type text (close #458) #1155

wants to merge 1 commit into from

Conversation

Anupam-dagar
Copy link
Contributor

This adds a toggle react component which can be used to change text input to textarea and vice versa.

close #458

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#458

Solution and Design

toggle_component

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@hasura-bot
Copy link
Contributor

Beep boop! 🤖

Hey @Anupam-dagar, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible. 🕐

Stay awesome! 😎

@praveenweb
Copy link
Member

@Anupam-dagar - The functionality seems to be fine. But we need some work on the UI. The toggle cannot be a big button with Toggle text on it. We can use some font-awesome icon for expand/collapse that is not intrusive. Also the new functionality seems to have broken some Insert rows test on Cypress. It would be nice if you could check them and fix.

@dsandip - thoughts on the UI suggestion?

@praveenweb praveenweb added c/console Related to console s/do-not-merge Do not merge this pull request to master labels Dec 3, 2018
@Anupam-dagar
Copy link
Contributor Author

@praveenweb I'll fix the tests.
Also for the font awesome icons, maybe we can use the following icons.
https://fontawesome.com/icons/text-width?style=solid
https://fontawesome.com/icons/text-height?style=solid
https://fontawesome.com/icons/align-justify?style=solid

@Anupam-dagar Anupam-dagar force-pushed the toggle_component branch 3 times, most recently from 23b3844 to 016a737 Compare December 10, 2018 12:29
@hasura-bot
Copy link
Contributor

Review app for commit 016a737 deployed to Heroku: https://hge-ci-pull-1155.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1155-016a737

@hasura-bot
Copy link
Contributor

Review app for commit 3b623bf deployed to Heroku: https://hge-ci-pull-1155.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1155-3b623bf

@praveenweb
Copy link
Member

@dsandip - can you suggest the final icon / position UI for the toggle functionality?

@dsandip
Copy link
Member

dsandip commented Dec 17, 2018

@shahidhk is the right person to take this call. I am not sure if just an icon like this will work and we may have to add text (text area ?)

@shahidhk
Copy link
Member

shahidhk commented Dec 17, 2018 •
edited
Loading

What about something like this?

expand_ui

Icon used is https://fontawesome.com/icons/expand?style=solid

We can also use https://fontawesome.com/icons/expand-alt?style=solid and https://fontawesome.com/icons/compress-alt?style=solid to indicate two states.

@Anupam-dagar
Copy link
Contributor Author

@shahidhk does the placement of icon remains same for text area?

@shahidhk
Copy link
Member

It has to be inside the textbox, top right corner.

@hasura-bot
Copy link
Contributor

Review app for commit 8cdad6a deployed to Heroku: https://hge-ci-pull-1155.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1155-8cdad6a

@hasura-bot
Copy link
Contributor

Review app for commit 52d0f2c deployed to Heroku: https://hge-ci-pull-1155.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1155-52d0f2c

praveenweb
praveenweb previously approved these changes Dec 20, 2018
View reviewed changes
@praveenweb praveenweb added s/ok-to-merge Status: This pull request can be merged to master and removed s/do-not-merge Do not merge this pull request to master labels Dec 20, 2018
@shahidhk shahidhk changed the title Allow multiline mode for input with type text. close #458 allow multiline mode for input with type text (close #458) Dec 21, 2018
shahidhk
shahidhk requested changes Dec 21, 2018
View reviewed changes
Copy link
Member

@shahidhk shahidhk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If type long text into the textbox and click the toggle button, textarea appears, but the text disappears.

@praveenweb @Anupam-dagar

@mnlbox
Copy link
Contributor

mnlbox commented Feb 13, 2019

@shahidhk any plan to merge this?

@Anupam-dagar
Copy link
Contributor Author

@shahidhk I pass the tests on this in sometime.

@mnlbox mnlbox mentioned this pull request May 15, 2019
Closed
@AnnieAtHasura
Copy link
Contributor

What about something like this?

expand_ui

Icon used is https://fontawesome.com/icons/expand?style=solid

We can also use https://fontawesome.com/icons/expand-alt?style=solid and https://fontawesome.com/icons/compress-alt?style=solid to indicate two states.

That icon's not obvious to me. I'll get design from Anu when I work on this.

@AnnieAtHasura AnnieAtHasura mentioned this pull request Jul 10, 2019
Merged
1 task
@rikinsk-zz
Copy link

Closing in favour of #2498

@rikinsk-zz rikinsk-zz closed this Jul 22, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1155.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Sep 25, 2024
@rakeshkky @hasura-bot
Test execute_request from graphql_ws crate (#1155)
601771c 
<!-- The PR description should answer 2 important questions: -->

### What

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

<!-- Does this PR introduce new validation that might break old builds?
-->

<!-- Consider: do we need to put new checks behind a flag? -->
This PR enables "unit" testing for `execute_request` function from
`graphql-ws` crate which is responsible for executing
graphql operations. It is tested in conjunction with the
`graphql_frontend`'s `execute_query` by comparing responses from the
both.

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->
Briefly
```
http_response = graphql_frontend::execute_query
ws_response = graphql_ws::execute_request
compare(http_response, ws_response)
```
V3_GIT_ORIGIN_REV_ID: 371ac3de1136732d7dc2e88dbd093264a96a7d2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahidhk shahidhk shahidhk requested changes

@praveenweb praveenweb praveenweb left review comments

Assignees
No one assigned
Labels
c/console Related to console s/ok-to-merge Status: This pull request can be merged to master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow multiline mode for input with type text
8 participants
@Anupam-dagar @hasura-bot @praveenweb @dsandip @shahidhk @mnlbox @AnnieAtHasura @rikinsk-zz