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 integrations screen #40

Merged
merged 30 commits into from
Mar 8, 2022
Merged

Add integrations screen #40

merged 30 commits into from
Mar 8, 2022

Conversation

Shelob9
Copy link
Contributor

@Shelob9 Shelob9 commented Mar 1, 2022

for #32

  • Integration screen
  • Helpscout integration, that can be toggled on and off
  • Helpscout configuration modal. Open by clicking helpscout logo
  • Come up with better way to open modal.
  • Use real values for secret/callback
  • Green button link
  • Buttons got kinda wonky
  • Make having one integration on integration screen not look bad

@Shelob9 Shelob9 changed the title WIP add integrations screen Add integrations screen Mar 3, 2022
@Shelob9 Shelob9 requested a review from zackkatz March 3, 2022 00:10
@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 3, 2022

@zackkatz A note: instead of copying how the modal is implimented in the design /trustedlogin-html-revisions-02/helpdesk-configure . I am going to use this: https://headlessui.dev/react/dialog and make the modal look like the design. Figuring out how to translate the CSS for the modal open/ close to React and also making it accessible is going to be a lot more work then using the component Tailwind built for this.

@zackkatz
Copy link
Contributor

zackkatz commented Mar 3, 2022

Sure, makes sense @Shelob9

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 3, 2022

@zackkatz This isn't done, but is close, see the todos I added to description. I'm done for today. If you have a chance to try this out and can you please let me know what you think:

  • How should this modal get opened? Currently need to click helpscout logo, values will be different per team. Should be part of team UI?
  • Looks good? Probably should have use Headless Ui before, it helped a lot.

@zackkatz
Copy link
Contributor

zackkatz commented Mar 4, 2022

@Shelob9 Looks good as a starting point. A few design issues (I figured this is a Work In Progress), but I figured I should point them out.


How should this be opened?

According to the design, it should be opened with a Configure button.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 4, 2022

@zackkatz I am not using that "configure" link from the design beacuse that would open the Global settings for Helpscout. There is no such setting.

I think this is an artifact of the previous plugin's data structure, which only had one team. This seams to be a constant issue: designs are for the previous data model (one account per plugin, global helpdesk settings) vs current data model: multiple teams, helpdesk settings are part of account settings.

The modal in the design has callback URL and secret, those are specific are specific to to a team.

I need to add these inputs to the Team UI, there is no design for that.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 4, 2022

Simplest solution, in my mind is to add a "Configure" button to the team

Screenshot 2022-03-04 152015

@zackkatz
Copy link
Contributor

zackkatz commented Mar 4, 2022

@Shelob9 Let's do that.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 4, 2022

@zackkatz Thumbs up emoji.

  • 44aae6f Limits helpdesks to only enabled helpdesks.
  • 8c944e9 Adds reset keys.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 4, 2022

@zackkatz Lingering questions:

  • Green button: where does it link to?
  • Do you have a theory on how to fix the buttons, I copied as is, something gave it margin top maybe?

@zackkatz
Copy link
Contributor

zackkatz commented Mar 7, 2022

@Shelob9 Don't worry about the modal button design for now.

What do you mean by the "green button"? If you mean the "Status: Connected" icon, it should open the connection summary.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 7, 2022

button

@Shelob9
Copy link
Contributor Author

Shelob9 commented Mar 8, 2022

@zackkatz I opened #44 and #45 for the unresolved issues and am now merging so I can finishing fixing #34 / move on.

@Shelob9 Shelob9 merged commit 6a7338a into main Mar 8, 2022
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.

2 participants