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

Changed image links #226

Closed
wants to merge 10 commits into from
Closed

Changed image links #226

wants to merge 10 commits into from

Conversation

clr-li
Copy link
Contributor

@clr-li clr-li commented Dec 4, 2023 •
edited
Loading

Asana: https://app.asana.com/0/1203067047205953/1205998067797202

Q/A checklist

  • Do a code review of the changes
  • Add any new dependencies to poetry & export to requirementst.txt (poetry export -o requirements.txt)
  • Carefully think about the stuff that might break because of this change
  • The relevant pages still run when you press submit
  • If you added new settings / knobs, the values get saved if you save it on the UI
  • The API for those pages still work (API tab)
  • The public API interface doesn't change if you didn't want it to (check API tab > docs page)
  • Do your UI changes (if applicable) look acceptable on mobile?

@clr-li clr-li requested a review from devxpy December 4, 2023 17:52
@clr-li
Copy link
Contributor Author

clr-li commented Dec 4, 2023

The images now only link to the workflow but it's a bit finicky. I tried the method similar recommending workflows method but the formatting was weird even when adding the classes

devxpy
devxpy reviewed Dec 11, 2023
View reviewed changes
explore.py Outdated
@@ -29,23 +30,23 @@ def _render_as_featured(page_cls):
page = page_cls()
state = page.recipe_doc_sr().to_dict()
# total_runs = page.get_total_runs()
render_image(page, state)
with gui.link(to=page.app_url()):
Copy link
Member

Choose a reason for hiding this comment

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

use href as per below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

devxpy
devxpy reviewed Dec 13, 2023
View reviewed changes
explore.py Outdated
@@ -37,15 +37,15 @@ def render_image(page, state):
gui.image(
page.get_recipe_image(state),
href=page.app_url(),
style={"border-radius": 5},
style={"border-radius": 5, "pointer-events": "none"},
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@devxpy devxpy mentioned this pull request Dec 13, 2023
Merged
8 tasks
@clr-li clr-li requested a review from devxpy December 13, 2023 18:31
@devxpy
Copy link
Member

devxpy commented Dec 14, 2023

Already included in https://github.com/GooeyAI/gooey-server/pull/228/files?

@devxpy devxpy closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devxpy devxpy Awaiting requested review from devxpy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clr-li @devxpy