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 Orchestral Case Study page to StackStorm website and add a Partner page URL #11

Merged
merged 13 commits into from
Aug 24, 2022
Merged

Add Orchestral Case Study page to StackStorm website and add a Partner page URL #11

merged 13 commits into from
Aug 24, 2022

Conversation

dalesmith
Copy link
Contributor

This PR is to merge changes that add an Orchestral Case Study page to the StackStorm website as well as place a link to that Case Study page on the StackStorm Partner page.

@dalesmith
Updated index.html
b2d20d7 
This is just a simple modification to the div containing the Orchestral StackStorm badges to add a link to the Orchestral Case Study page.
@dalesmith
Added folder and index.html file
7a6d58f 
Added folder an index.html for Orchestral Case Study page
arm4b
arm4b reviewed Aug 8, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

This PR version is good at handling the diff 👍

@dalesmith
Update index.html
66dd268 
Fixed the orchestral.local URL problem and double checked all URLs generally. 
Also, removed some unnecessary script tags.
@dalesmith
Added all images to the PR
2103c77 
An "images" sub-directory has been added and all image files have been added. 
Additionally, all links on the Orchestral Case Study page have been adjusted to point to images in this sub-directory.
@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 9, 2022 •
edited
Loading

All image files have been placed into an "images" sub-directory to the index.html directory.
Thus, all image files are now within this PR with no externally referenced images and all URLs have been adjusted accordingly.
@armab Thanks for the very helpful guidance!...much appreciated. :-)

@dalesmith dalesmith requested a review from arm4b August 9, 2022 05:11
arm4b
arm4b suggested changes Aug 9, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @dalesmith,
Left a comment about the images still referring to orchestral.ai as an external source. Needs fixing.

You might also need to add a new Orchestral block here: https://stackstorm.com/case-studies/ to get discovered more easily.

Outside of that, I liked the "Symfony" naming, good stuff 👍

case-study-orchestral/index.html Outdated Show resolved Hide resolved
case-study-orchestral/index.html Outdated Show resolved Hide resolved
@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 10, 2022 via email

arm4b
arm4b suggested changes Aug 10, 2022
View reviewed changes
case-study-orchestral/index.html Outdated Show resolved Hide resolved
case-study-orchestral/index.html Outdated Show resolved Hide resolved
case-study-orchestral/index.html Outdated Show resolved Hide resolved
case-study-orchestral/index.html Outdated Show resolved Hide resolved
case-study-orchestral/index.html Outdated Show resolved Hide resolved
@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 10, 2022 via email

@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 10, 2022 via email

arm4b
arm4b suggested changes Aug 11, 2022
View reviewed changes
case-study-orchestral/index.html Outdated Show resolved Hide resolved
@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 12, 2022 via email

@dalesmith
Update index.html
f0bb5db 
Did thorough scrub to remove all external references to asset files, i.e. js, css. 
Also, changed all image URLs to absolute dir/path structure.
@dalesmith dalesmith requested a review from arm4b August 15, 2022 15:21
@dalesmith
Copy link
Contributor Author

Hi Eugen,
Thanks for your patience and guidance here.
I hope that I have fixed everything per your recommendations.
So, fingers crossed...this time the PR can go through but don't hesitate to let me know if there are additional fixes required.

arm4b
arm4b reviewed Aug 16, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @dalesmith, the page HTML looks good, - great work on polishing it!

The only missing part now is an Orchestral block on the https://stackstorm.com/case-studies/ page, which will make your case study more discoverable.
That'll make it complete and ready for publishing.

@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 16, 2022 via email

@dalesmith
Update index.html
f51f6f0 
This change adds an Orchestral.ai "block" to the Case Study homepage on the StackStorm website.
@dalesmith
Added image file - DSmith.png
43326a4 
This change adds an image file - DSmith.png for use in the Orchestral block added to the Case Study homepage of the StackStorm site.
@dalesmith dalesmith requested a review from arm4b August 17, 2022 20:11
arm4b
arm4b suggested changes Aug 17, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @dalesmith, the new content block looks good 👍
image

However, as a final check I just found that after you removed some CSS references the case study looks this way that's definitely broken:

image

Looks like the page was referring to some CSS styles that existed on orchestral.ai, but not on stackstorm.com. The previous version (before f0bb5db) looked OK.

Here are the 3 CSS files that are expected by your HTML:

<link rel="stylesheet" href="https://orchestral.ai/wp-content/plugins/oxygen/component-framework/oxygen.css?ver=4.0.2" type="text/css" media="all"/>
<link rel="stylesheet" href="//orchestral.ai/wp-content/uploads/oxygen/css/2332.css?cache=1658928015&#038;ver=6.0.1" type="text/css" media="all"/>
<link rel="stylesheet" href="//orchestral.ai/wp-content/uploads/oxygen/css/universal.css?cache=1658936364&#038;ver=6.0.1" type="text/css" media="all"/>

Try to refactor the HTML so it doesn't rely on external CSS references but uses existing stackstorm styles.

Probably copy-pasting the HTML body structure from the other case studies and replacing only the text + images would be the easiest path.

@arm4b arm4b added the partners label Aug 17, 2022
@arm4b
Copy link
Member

arm4b commented Aug 17, 2022

FYI I'm also adding README instructions for the local development with docker-compose and nginx here: #12, if that helps.

@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 18, 2022 via email

@dalesmith
Added assets sub-directory with (3) css files.
bf0e40a 
Added "assets" subdirectory containing (3) css files as local assets.
@dalesmith
Added references to missing css as local assets.
2485422 
At line (263), added references to the (3) missing css files in the "assets" sub-directory. Also, tested this change with localhost to any more embarrassing oversights on my part. ;-)
@dalesmith
Copy link
Contributor Author

I tested these changes with localhost so hopefully (fingers crossed) this PR will not be troublesome any longer.
Thanks for the great guidance and assistance on this @eugen, much appreciated!

@dalesmith dalesmith requested a review from arm4b August 18, 2022 19:05
arm4b
arm4b reviewed Aug 18, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @dalesmith, sorry we can't add 3 new CSS files from the other website because the case study relies on it, otherwise, the stackstorm.com would become inconsistent very quickly.

Instead, the future direction is to remove all the unneeded CSS styles from the WordPress leftower (see Cleanup and Merge CSS #8 issue for more context).

Please try to refactor the HTML so it uses the existing stackstorm styles, like the other case studies.
As suggested before, copy-pasting the HTML body structure from the other case studies and replacing only the text + images with your content would be the easiest path.

I hope that makes sense in the context of the stackstorm.com website consistency we maintain collectively now.

@dalesmith
Updated index.html for Orchestral Case Study page
be0b93d 
This Commit includes changes that remove the prior use of non-StackStorm CSS in the index.html for the Orchestral Case Study page. 
Additionally, this Commit includes changes to the index.html for the StackStorm Thought Leaders page to add an Orchestral entry to the page.
@dalesmith dalesmith requested a review from arm4b August 22, 2022 07:15
@dalesmith
Copy link
Contributor Author

I think these last set of changes will do it.
As always, don't hesitate to let me know if further work is required.

arm4b
arm4b reviewed Aug 22, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @dalesmith, thanks for the changes, the main case study HTML page looks good now.

Could you please also delete assets/2332.css, assets/universal.css, and assets/universal.css files? They're not used anymore but are still included in the git.

Outside of that minor file cleanup, everything looks good to me 👍

@dalesmith
Copy link
Contributor Author

dalesmith commented Aug 23, 2022 via email

@dalesmith
Delete case-study-orchestral/assets directory
e62042d 
Neglecting to remove this directory is a minor oversight from the last Commit. These asset files are not used/needed.
@dalesmith
Copy link
Contributor Author

Deleted the 'assets' directory containing (3) css files that are not needed/used.

@dalesmith dalesmith requested a review from arm4b August 23, 2022 01:51
arm4b
arm4b approved these changes Aug 23, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Thanks a lot @dalesmith for all the changes, great work!

@arm4b arm4b requested a review from a team August 23, 2022 11:39
@arm4b arm4b enabled auto-merge August 24, 2022 11:57
nzlosh
nzlosh approved these changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@nzlosh nzlosh left a comment

Choose a reason for hiding this comment

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

LGTM

@arm4b arm4b merged commit d74f874 into StackStorm:main Aug 24, 2022
@dalesmith dalesmith deleted the add-orchestral-case-study-page-and-partner-page-url branch August 24, 2022 14:03
@dalesmith
Copy link
Contributor Author

dalesmith commented Oct 11, 2022 via email

@arm4b
Copy link
Member

arm4b commented Oct 11, 2022

Hey @dalesmith, the comment you're mentioning was resolved a while ago.
The PR was merged almost 2 months ago, no other changes are needed.
So no worries!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nzlosh nzlosh nzlosh approved these changes

@arm4b arm4b arm4b approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@dalesmith @arm4b @nzlosh