这是indexloc提供的服务，不要输入任何密码
Skip to content

console: fix listen update column config selection for event trigger (close #5042) #5043

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
Merged
merged 3 commits into from
Jun 10, 2020
Merged

console: fix listen update column config selection for event trigger (close #5042) #5043

merged 3 commits into from
Jun 10, 2020

Conversation

@wawhal
Copy link
Contributor

@wawhal wawhal commented Jun 10, 2020 •
edited by rikinsk
Loading

Description

Check out #5042
Also I noticed some inconsistencies in the checkboxes: cursor pointer, label not clickable, unnecessary margins. Fixed these as well.

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System
  • Tests
  • Other (list it)

Related Issues

#5042

@wawhal wawhal requested a review from a team as a code owner June 10, 2020 10:52
@wawhal wawhal requested a review from a team as a code owner June 10, 2020 10:53
@rikinsk rikinsk added the c/console Related to console label Jun 10, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Jun 10, 2020

Review app for commit bb291cc deployed to Heroku: https://hge-ci-pull-5043.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5043-bb291ccf

@rikinsk rikinsk changed the title console: fix column config in event trigger (closes #5042) console: fix listen update column config selection for event trigger (close #5042) Jun 10, 2020
rikinsk
rikinsk approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@rikinsk rikinsk left a comment

Choose a reason for hiding this comment

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

UX and Changelog approved

@rikinsk rikinsk requested a review from beerose June 10, 2020 11:19
@rikinsk rikinsk removed their assignment Jun 10, 2020
beerose
beerose reviewed Jun 10, 2020
View reviewed changes
Copy link
Contributor

@beerose beerose left a comment

Choose a reason for hiding this comment

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

Code looks okay.

I noticed one small issue that there's no pointer cursor for the column checkbox, only for the label. Not sure what's the issue here, the class is added correctly.

2020-06-10 at 13 38 22 - Crimson Tiglon

When I tried applying the same class names as operation checkboxes it seemed to be fixed.

@rikinsk
Copy link
Member

rikinsk commented Jun 10, 2020

@beerose Maybe we can ignore this for now? All these inconsistencies should go away once we do the refactor to use the common checkbox component everywhere. Till then we are always going to keep finding these inconsitencies.

@beerose
Copy link
Contributor

beerose commented Jun 10, 2020

Makes sense, that's why I only left a comment. Will approve this.

@beerose beerose merged commit 63eb311 into hasura:master Jun 10, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Jun 10, 2020

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

beerose pushed a commit that referenced this pull request Jun 10, 2020
console: fix listen update column config selection for event trigger (c…
f818237 
…lose #5042) (#5043)
stevefan1999-personal pushed a commit to stevefan1999-personal/graphql-engine that referenced this pull request Sep 12, 2020
@stevefan1999-personal
console: fix listen update column config selection for event trigger (c…
36bd4da 
…lose hasura#5042) (hasura#5043)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rikinsk rikinsk rikinsk approved these changes

+1 more reviewer

@beerose beerose beerose approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@beerose beerose

Labels

c/console Related to console

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wawhal @hasura-bot @rikinsk @beerose