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

[FEAT] Add link to index.html when clicking the PA logo #251

Merged
merged 13 commits into from
Oct 26, 2021
Merged

[FEAT] Add link to index.html when clicking the PA logo #251

merged 13 commits into from
Oct 26, 2021

Conversation

kylejohnston
Copy link
Contributor

@kylejohnston kylejohnston commented Oct 18, 2021

closes #245

All examples

  • Added link to index.html when clicking the PA logo

Homepage

  • Removed "_blank" from links
  • Improved responsive layout

@CLAassistant
Copy link

CLAassistant commented Oct 18, 2021

CLA assistant check
All committers have signed the CLA.

@tbouffard tbouffard added the enhancement New feature or request label Oct 18, 2021
@tbouffard
Copy link
Member

Thanks @kylejohnston for your contribution. I will review it soon.

@tbouffard tbouffard added hacktoberfest-accepted Accepted Pull Request during Hacktoberfest external contribution 👤 Pull requests provided by someone who is not a core maintainer labels Oct 18, 2021
@tbouffard tbouffard self-requested a review October 18, 2021 14:46
@tbouffard
Copy link
Member

tbouffard commented Oct 19, 2021

✔️ Responsive layout improvements on the examples home page (00cbc9a)

Better title height and section title not cropped when accessed by anchors

0.19.3 Now
image image

Single card displayed on small screens
width <= 840px
⏩ commit with statically.io

0.19.3 Now
image image

Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. It looks great. I only have minor change requests.
✔️ the responsive layout improvements work great, see #251 (comment)
❌ the href to the examples home page should use relative path to work with local file browsing and statically.io

examples/index.html Outdated Show resolved Hide resolved
examples/index.html Outdated Show resolved Hide resolved
Comment on lines 18 to 21
<a class="home flex items-center" href="/examples/index.html">
<img src="static/img/logo_64x64_white.png" alt="logo" class="logo h-11 m-2 ml-3">
<span class="h-11 m-2 py-1 text-3xl">BPMN Visualization</span>
</a>
Copy link
Member

@tbouffard tbouffard Oct 18, 2021

Choose a reason for hiding this comment

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

ℹ️ The content here is copied during release of https://github.com/process-analytics/bpmn-visualization-js/ so it will be erased next time we update it like in #252.
This wasn't mentioned in the issue, so feel free to revert or keep it. We will handle it later in the bpmn-visualization-js repository.

demo/hacktoberfest-custom-themes/index.html Outdated Show resolved Hide resolved
examples/custom-behavior/apply-css-classes/index.html Outdated Show resolved Hide resolved
examples/index.html Outdated Show resolved Hide resolved
@tbouffard tbouffard changed the title Issue 245 fixes [FEAT] In all examples, add link to index.html when clicking the PA logo Oct 19, 2021
@kylejohnston
Copy link
Contributor Author

Just made new commits that address the requested changes. Let me know if there's anything else!

examples/index.html Outdated Show resolved Hide resolved
examples/index.html Outdated Show resolved Hide resolved
examples/index.html Outdated Show resolved Hide resolved
Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

@kylejohnston thanks for implementing the requested changes.
I have directly pushed some small changes to finalize the Pull Request. It was faster (and less annoying for you) to make them myself rather than commenting and asking you to make the changes 😄.
I'll leave the Pull Request open for a few days for you to check if there are any issues with my edits. Then I will merge it.

❓ I have one last question because I am curious 😸. I would like to know how you found out about our repository. Do you work in the field of BPMN, Process Analytics or Process Mining? or did you just make a contribution as part of Hacktoberfest for instance?
Anyway, thanks again for your contribution.

@kylejohnston
Copy link
Contributor Author

Thank you for the update and the opportunity to contribute! I do not work in this field; I searched for repos with design / frontend issues tagged with hacktoberfest.

Open in a new tab as we have no way to go back to home from the examples hosted externally
@tbouffard
Copy link
Member

@tbouffard tbouffard changed the title [FEAT] In all examples, add link to index.html when clicking the PA logo [FEAT] Add link to index.html when clicking the PA logo Oct 26, 2021
@tbouffard tbouffard merged commit 673d320 into process-analytics:master Oct 26, 2021
@kylejohnston kylejohnston deleted the issue-245-fixes branch October 28, 2021 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external contribution 👤 Pull requests provided by someone who is not a core maintainer hacktoberfest-accepted Accepted Pull Request during Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] In all examples, go to the index.html when clicking the PA logo
3 participants