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

Add optional image and caption attributes to Connector and Cable/Bundle nodes #137

Closed
wants to merge 17 commits into from
Closed

Add optional image and caption attributes to Connector and Cable/Bundle nodes #137

wants to merge 17 commits into from

Conversation

kvid
Copy link
Collaborator

@kvid kvid commented Jul 30, 2020 •
edited
Loading

@formatc1702
Copy link
Collaborator

Nice. You could build upon #136 and set feature/gv-html-refactor as the base branch to take advantage of the refactored code for cables :)

@kvid kvid mentioned this pull request Jul 30, 2020
Open
@formatc1702 formatc1702 added this to the v0.3 or later milestone Aug 2, 2020
@kvid kvid changed the title Add an image attribute to both Connectors and Cables Add optional image and caption attributes to Connector and Cable/Bundle nodes Aug 4, 2020
@kvid kvid mentioned this pull request Aug 4, 2020
Merged
@kvid
Copy link
Collaborator Author

kvid commented Aug 4, 2020

Nice. You could build upon #136 and set feature/gv-html-refactor as the base branch to take advantage of the refactored code for cables :)

This rebase is not yet done, as it depends on your answer(s) to my #136 (comment) about styling, but this PR is ready for testing the functionality.

Finalizing this PR also depends on your answer to my #27 (comment) that also might require a few changes.

@kvid kvid marked this pull request as ready for review August 4, 2020 18:11
@kvid kvid mentioned this pull request Aug 4, 2020
Open
formatc1702 and others added 9 commits August 10, 2020 16:44
@formatc1702
Add suggested changes
eefea24
@kvid
Add an image attribute to both Connectors and Cables
cf57f0e
@kvid
Add a caption attribute to both Connectors and Cables
50782fd
@kvid
Remove border between image and caption
1d3e7e4 
Adding helper function for each of these with a leading <tdX> tag that
instructs nested_html_table() to inject attributes to the <td> tag.
@kvid
Expand example 08 with images and captions
00b4d72 
- Stereo phone plug (a slight modification of a public domain image
  from https://openclipart.org/detail/192396/headphones-connctor ).
- Cable cross-section drawn to match the wire colors in example 08.
- Make the cable shield color match the one in the cross-section.
- Images for embedding in the connector and cable nodes are stored
  in a new resources folder.
@kvid
Add an image_scale attribute to both Connectors and Cables
335f421 
It specifies how an image will use any extra space available in its
cell. Allowed values are one of these strings:
- FALSE : keep image its natural size. (Default)
- TRUE : scale image uniformly to fit.
- WIDTH : expand image width to fill.
- HEIGHT : expand image height to fill.
- BOTH : expand both image width and height to fill.

The value is sent to Graphviz as a scale attribute to the <img> tag.
Note that there is normally no extra height in the image cell.
@kvid
Add an image_size attribute to both Connectors and Cables
b1173a0 
It is a list containing minimum width and minimum height of the
image cell. To obtain more available space in the image cell,
this size must be set greater than the natural size of the image.
@kvid
Add fixedsize as third image_size value
6081bf5 
When True, enclose the image cell in a table without borders to avoid
narrow borders when the fixed width is less than the node width.
@kvid
Simplify colorbar using the same technique as html_image()
9c933d1 
Moving common code into html_colorbar() helper function.
@kvid kvid changed the base branch from dev to feature/gv-html-refactor August 12, 2020 17:31
@kvid kvid mentioned this pull request Aug 13, 2020
Closed
@formatc1702
Copy link
Collaborator

formatc1702 commented Aug 13, 2020 •
edited
Loading

@kvid if you are wondering why this is marked as closed:
I did not actively close this PR; but this is what I feared would happen when I wrote my comment in #136.
The base branch no longer exists after the rebasing of #136, and GitHub has automatically closed the PR because of that.
You probably have two options now:

  • Edit this PR to point to the dev branch instead of feature/gv-html-refactor. Perhaps you can then reopen the PR.
  • Open a new PR based on dev and submit the changes there.

Please let me know how you want to proceed!

@kvid kvid mentioned this pull request Aug 13, 2020
Merged
@kvid
Copy link
Collaborator Author

kvid commented Aug 13, 2020

@formatc1702 wrote:

@kvid if you are wondering why this is marked as closed:
I did not actively close this PR; but this is what I feared would happen when I wrote my comment in #136.
The base branch no longer exists after the rebasing of #136, and GitHub has automatically closed the PR because of that.

I'm sorry, I misunderstood your rebase plan. I thought you were planning an interactive rebase to fix the commit history and keep the feature branch - not merge it into dev and kill the feature branch.

You probably have two options now:

  • Edit this PR to point to the dev branch instead of feature/gv-html-refactor. Perhaps you can then reopen the PR.

I could edit the PR, but only the name, not the base branch.

If I had understood you would delete your feature branch, then I would have changed the base branch of my PR before your deleted your feature branch.

  • Open a new PR based on dev and submit the changes there.

I have already created PR #153 that is now ready for review.

@formatc1702
Copy link
Collaborator

Ahh, I see.
Usually, after a feature is merged into dev (doesn't matter whether merge/rebase/squash), the feature branch is no longer needed, and since you approved the changes, that's what happens. Sorry if it was unexpected!
Anyway, thanks for the new PR. Will take a look in the next few days.

formatc1702 pushed a commit that referenced this pull request Oct 14, 2020
@kvid
Add optional image to connectors and cables (#153)
4782da4 
This image, with an optional caption below, is displayed in the lower 
section of the connector/cable node in the diagram - just above the 
notes if present.

This solves the basic part of issue #27, and is a continuation of 
PR #137 that was closed due to changes in the base branch.
@formatc1702 formatc1702 modified the milestones: v0.3, v0.2 Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.2
Development

Successfully merging this pull request may close these issues.
2 participants
@kvid @formatc1702