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 subtitles, popup and info around Applications/Instances #2177

Merged
merged 2 commits into from
May 30, 2023

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented May 24, 2023

closes #1995

Description

  • Add missing subtitles
  • Add info button and popup
  • Ensure texts align with current implementation

image

image

Related Issue(s)

#1995

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Steve-Mcl
Copy link
Contributor Author

@knolleary PR in draft as I suspect UI folk may want to review/adjust icons

Feel free to switch to release if necessary.

@Steve-Mcl Steve-Mcl requested a review from knolleary May 24, 2023 08:23
@knolleary
Copy link
Member

@Steve-Mcl if you want it to be reviewed, then mark it ready and add the appropriate people as reviewers. As it stands there's no indication there's any action outstanding with anyone other than you.

@Steve-Mcl Steve-Mcl requested a review from joepavitt May 24, 2023 12:27
@Steve-Mcl Steve-Mcl marked this pull request as ready for review May 24, 2023 12:27
@Steve-Mcl
Copy link
Contributor Author

@joepavitt I have marked this ready for review and added you as a reviewer. I suspect you will want to evaluate the images used in the dialogs . I realise you are on vacation (so my apologies if you have notifications firing off - hopefully muted)

@Yndira-FlowForge I have tagged you for the same reason - on behalf of Joe, you may wish to review the iconography in his absence.

@Steve-Mcl
Copy link
Contributor Author

@knolleary you are requested as reviewer because you raised the issue and I would like you to review the text content of this PR.

@Yndira-E
Copy link
Contributor

@Steve-Mc I think we could use the ones we created for the empty states, on these two and on library and devices as well (I noticed those have the same icon)
I just need to color them because the ones we have are all gray, could this wait until tomorrow morning?

@Steve-Mcl
Copy link
Contributor Author

I just need to color them because the ones we have are all gray, could this wait until tomorrow morning?

of course.

@Yndira-E Yndira-E linked an issue May 25, 2023 that may be closed by this pull request
@Yndira-E
Copy link
Contributor

Hi @Steve-Mcl the images you need are in the drive, these are the ones you should use:
Screenshot 2023-05-25 at 12 15 14

@Steve-Mcl
Copy link
Contributor Author

Hi @Steve-Mcl the images you need are in the drive, these are the ones you should use: Screenshot 2023-05-25 at 12 15 14

That is fab, thank you very much.

@Yndira-E
Copy link
Contributor

@Steve-Mcl just added two more, in case you want to replace the one we have in snapshots now and if we need one for pipelines too.
Screenshot 2023-05-26 at 13 25 43

@Steve-Mcl
Copy link
Contributor Author

Steve-Mcl commented May 26, 2023

@Yndira-FlowForge I can certainly include them when I get back to this PR (later today or Monday) - are you/Joe happy for me to update the pipeline and snapshot images in this PR (not too tenuous in regards to the original issue raised?)

@Yndira-E
Copy link
Contributor

You're right @Steve-Mcl this should be a new issue. I just created one for this. Thanks for pointing it out ;)

@Steve-Mcl Steve-Mcl marked this pull request as draft May 30, 2023 07:49
@joepavitt
Copy link
Contributor

@Steve-Mcl what's the status here? Any particular reason it's still in draft?

@Steve-Mcl
Copy link
Contributor Author

@joepavitt switched back to draft so this doesn't get inadvertently merged without updating the popup graphics with those kindly provided by Yndira.

And the only reason I have not yet come back to this is because my dev-env is kinda a mess while I develop another feature.

Should be updated before end of today Joe & I will switch back to release.


in short, I need to add the new graphics:

Hi @Steve-Mcl the images you need are in the drive, these are the ones you should use:
Screenshot 2023-05-25 at 12 15 14

... and apply them to the popups to finish this off.

@joepavitt
Copy link
Contributor

Thanks Steve - I'll take this and update the images

@joepavitt joepavitt marked this pull request as ready for review May 30, 2023 10:04
@joepavitt
Copy link
Contributor

failing tests are out of scope for this PR, they're fixed in: https://github.com/flowforge/flowforge/pull/2195/files

@joepavitt joepavitt merged commit 55b61ff into main May 30, 2023
@joepavitt joepavitt deleted the 1995-Application-Instances-info-popup branch May 30, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants