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

Added product screenshots #162

Merged
merged 1 commit into from
Jun 28, 2021
Merged

Added product screenshots #162

merged 1 commit into from
Jun 28, 2021

Conversation

JonasHelming
Copy link
Contributor

@JonasHelming JonasHelming commented May 21, 2021

fixed #160

Added slider for product screenshots, including details on click.

Adapted order of elements on page

Signed-off-by: Jonas Helming [email protected]

@JonasHelming JonasHelming force-pushed the GH-160 branch 11 times, most recently from 05b3579 to 592003f Compare May 21, 2021 20:59
@JonasHelming JonasHelming force-pushed the GH-160 branch 2 times, most recently from 61fdce7 to d61f5e4 Compare June 9, 2021 07:54
@brianking
Copy link

I like the slider, 2 comments:

  • Will they be captioned with the product name?
  • At that size, they are lacking a certain level of detail

@JonasHelming
Copy link
Contributor Author

@brianking click on them and enjoy the magic!

@brianking
Copy link

Nice! It is not reliable for me yet, in that it mostly shows a different product than the one I click on. Also, it would be a better visual cue if the cursor changed when hovering on them, like a link.

@JonasHelming
Copy link
Contributor Author

Yes, this is know, the PR is WIP

@JonasHelming JonasHelming changed the title WIP - Added product screenshots Added product screenshots Jun 11, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@JonasHelming any reason why we changed the nice header of the existing website in order to add example product screenshots that make use of the framework?

Compared to the existing site:

  • the page feels more crowded and condensed, harder to read
  • the button placements are odd ('view on gitpod' and 'try now')
  • 'contributors and adopters' have less overall padding (after the header)
  • it would have been nice to have nicer application screenshots, the screenshots are often cropped or have extra background content which does not give a professional feeling

These are simply my nitpicks :)

@JonasHelming
Copy link
Contributor Author

@vince-fugnitto :
Thank your for your feedback!

@JonasHelming any reason why we changed the nice header of the existing website in order to add example product screenshots that make use of the framework?

Yes! The old header consists of a gigantic screenshot with zero message. I want to show visitor immediately and visually three core messages:

  1. Theia is a platform forr building tools
  2. The advantages listed below the gallery
  3. Theia is adopted by quite a few companies and products

I believe these messages are very important for visitors.

I am happy to discuss spacing and margins. However, I disagree that the old header was "nice". Or maybe "nice" but not really usful. A header that spans significantly over the first visible area (without scrolling) is not really a header. It indicates that Theia is a IDE, not a platform. I hope this makes sense to you.

Compared to the existing site:

  • the page feels more crowded and condensed, harder to read
    Yes, but we are coming from an extrem "undense" site. Happy to optimize the PR, but everyhing is more dense than a 1280*1024 screesnhot :-D
  • the button placements are odd ('view on gitpod' and 'try now')
    Could you describe more in detail what you find odd please?
  • 'contributors and adopters' have less overall padding (after the header)
    Done to save space. My goal was to have the "adopters" section still a little bit visible on a usual screen so people get interested and scoll down.
  • it would have been nice to have nicer application screenshots, the screenshots are often cropped or have extra background content which does not give a professional feeling
    Which onces do you dislike? We can ask for better once!

These are simply my nitpicks :)
As always appreciated!

@JonasHelming
Copy link
Contributor Author

Sorry, my comments are mixed in your reply in the comment above

@vince-fugnitto
Copy link
Member

@JonasHelming it might be better to link issues, and describe the changeset better in the pull-request description. We mention 'added product screenshots' but it is much more than that:

  • the original landing page is replaced with a smaller header without any real explanation
  • we include the screenshot carousel of example applications
  • what do we mean when we say 'selected tools based on eclipse theia'? should we instead say 'example theia-based applications'?

As for the padding, I'm referring to spacing between headers and their content (which makes the page feel too visually condensed:

image

image

Compared to master:

image

image

We should have consistent padding across different sections.

For accessibility we might want to make images in the carousel look clickable (change the cursor).

@JonasHelming JonasHelming changed the title Added product screenshots WIP - Added product screenshots Jun 15, 2021
@brianking
Copy link

In general I like it. The more compact header is nice and makes better use of space. A few feedback points:

  1. In the screenshots carousel, I wonder if the text “Selected Tools based on Eclipse Theia” be “Selected Products and Tools …“? Either way it works.
  2. For me, the small screenshots in the carousel are just a little too small. They are fine on a big monitor, but on smaller screens like my 13” laptop it is impossible to see much detail that differentiates them. I would consider making them bigger and just having two visible at a time.
  3. In my mobile browser (iOS Safari) the small carousel screenshots are not scaling correctly. The bigger versions when selected scale fine.

@JonasHelming
Copy link
Contributor Author

  • We fixed the "crouched" design in the header and the button placement, I believe it looks much better now.
  • We made the images change the cursor, so there is visual feedback that you can click them
  • An issue has been linked
  • The bottom margin has been increased and made consistent all across the site

There are several suggestion for the heading now, what about: "Selected example Theia-based Tools and IDEs" or "Selected Theia-based Tools and IDEs" or "Selected tools and IDEs based on Theia". I am unsure!

We will have a look at mobile version.
About the small screens: I tested it, and I would accept it. I think the still look like a variaty of tools due to different colors and that is the important message. For details, you can still click.

@JonasHelming
Copy link
Contributor Author

We reduced the gallery on mobile to show only one screenshot.

All comments aboved should be either fixed or commented on, please have a look if you want @vince-fugnitto @brianking

@brianking
Copy link

I suggest putting an x to close the pop up window. ESC or clicking out of it is not intuitive to everyone.

@JonasHelming JonasHelming changed the title WIP - Added product screenshots Added product screenshots Jun 17, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the changes look much better after recent updates and address concerns I had with the original updates.

Are we planning on addressing Brian's #162 (comment) regarding adding an x or close button for the dialogs?

I confirmed that:

  • the newly added dependencies are license compatible
  • the header is well positioned, and clear to read
  • the product screenshots are all displayed correctly, selecting more information correctly brings users to their respective homepages
  • the website remains responsive when changing the viewport size

src/utils/data.js Outdated Show resolved Hide resolved
@JonasHelming
Copy link
Contributor Author

I suggest putting an x to close the pop up window. ESC or clicking out of it is not intuitive to everyone.

Fixed

@JonasHelming
Copy link
Contributor Author

@vince-fugnitto : Are you good with merging this now?

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@vince-fugnitto : Are you good with merging this now?

The content looks fine, please squash first? (the history is quite messy)

Comment on lines +134 to +135

Copy link
Member

Choose a reason for hiding this comment

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

For additional information, the formatting of this file is incorrect which therefore results in a newline not being present at the end of the file. I won't nitpick for the moment, we can think of adding rules and formatting properly in a subsequent pull-request.

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 #160

- adapted order of elements on page
- Move logo to Nav
- Move gh-iframes to top-right, below Nav
- Display buttons next to text on big screens
- Show products in a Slider
- Show Popup on click on Product
- Introduce consistent header margins

Signed-off-by: Jonas Helming <[email protected]>
@JonasHelming
Copy link
Contributor Author

@vince-fugnitto : squashed!

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look fine to me 👍

@JonasHelming JonasHelming merged commit 1bd1321 into master Jun 28, 2021
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.

[content] Adopters - Product Screenshots
3 participants