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

Turned HTML version into self-saving document #36

Merged
merged 5 commits into from
Dec 5, 2022

Conversation

FND
Copy link
Contributor

@FND FND commented Nov 27, 2022

as suggested in #33

this is fairly crude, particulary because for now we only support text fields (both input[type=text] and textarea) and radio buttons

we might also use file-system APIs https://web.dev/file-system-access/ where supported, but that seems overkill for the moment

NB: This has not been heavily tested, so a thorough review seems necessary.

Copy link
Contributor

@yellowbrickc yellowbrickc left a comment

Choose a reason for hiding this comment

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

I tested it in chrome, it works fine. What do you think about adding the name of the context to the download link? (As for now, all downloads are called bounded-context-canvas.html.)

Other 2 points which could be improved on the Html version, if you want (regarding the content, not the self-saving functionality):

  1. at the bottom of the page, the references to the different relationships are incomplete.
  2. missing space between Source and DocumentationDDD Crew on GitHub: Bounded Context Canvas

tools/html-version/bounded-context-canvas-template.html Outdated Show resolved Hide resolved
Copy link
Contributor

@yellowbrickc yellowbrickc left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@FND
Copy link
Contributor Author

FND commented Dec 5, 2022

What do you think about adding the name of the context to the download link?

Excellent idea; done.

I've fixed some of the details you mentioned in 830e25e, but I'm a little uncertain about the list of relationships there - in part because I'm not quite clear on the original author's intent. So I've opted not to change any content here. (My guess is that the HTML version might be due for a little overhaul anyway? 🤷 )

this is fairly crude, particulary because for now we only support text
fields (both `input[type=text]` and `textarea`) and radio buttons

we might also use file-system APIs <https://web.dev/file-system-access/>
where supported, but that seems overkill for the moment
we do _not_ want to persist the `hidden` and `href` attributes' state

if this were any more complex, we might need proper encapsulation - but
for now that's YAGNI
plus proper spacing
@yellowbrickc
Copy link
Contributor

Hello @FND

I'm a little uncertain about the list of relationships there - in part because I'm not quite clear on the original author's intent. So I've opted not to change any content here. (My guess is that the HTML version might be due for a little overhaul anyway? 🤷 )

Yes, I agree, you did enough! Thank you.

@yellowbrickc yellowbrickc merged commit 94023d1 into ddd-crew:master Dec 5, 2022
@FND FND deleted the auto-saving-html branch December 6, 2022 07:25
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.

None yet

3 participants