Skip to content

Conversation

@ThatcherK
Copy link

Ticket #18

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ThatcherK ThatcherK requested a review from davidebukali July 6, 2023 10:10
@davidebukali
Copy link
Collaborator

Instead of changing the preview with fixed width we could use a class and pass it to the iframe; Maybe render iframe inside chartPreview or create a parent container for both. Also good to have ...
-Add toggle button to hide/show preview
-Option to adjust width of the preview window
-Make preview window height fixed like wagtail, both windows ought to scroll individually

@APIYOJENNIFER APIYOJENNIFER force-pushed the feature/different-screen-view branch from b7ca09e to 9452f6e Compare September 12, 2023 07:11
@ThatcherK
Copy link
Author

ThatcherK commented Sep 21, 2023

This is some good work so far!
Looking at behavioural review first

  • I would rather the default for the preview, is just having the checkbox that says show preview window unchecked.

  • Instead of letting a use enter a screen size, add the sizes via buttons with labels of desktop, ipad and so fourth. You could also use a dropdown, whichever you prefer

  • Not sure how wagtail does it, but they manage to scale the preview window, so that all the content fits within the preview pane without the need to scroll horizontally.

  • Let the show preview be more visible, Like something someone cannot miss when they look at the page

@ThatcherK
Copy link
Author

Much as the screen size of the preview window changes according to the different toggles . The view is quite confusing. It looks like the preview window is now on the left. Ideally, The main screen should be bigger than the preview window.
But let me show you a screen grab of how it looks in the same pane . Notice how the preview is showing the desktop view, within that small view and it all fits, without scrolling left.( I am sure they use some kind of scaling, to be able to fit what would rather need a bigger space, and still look the same as if it were occupying a bigger space).
The other option would be to open the preview in a different tab, but then there would be no value add because that is already provided by google dev tools.
Screenshot 2023-09-26 at 12 37 33

@APIYOJENNIFER APIYOJENNIFER force-pushed the feature/different-screen-view branch 2 times, most recently from 12fd3be to e90da1d Compare October 12, 2023 06:30
@APIYOJENNIFER APIYOJENNIFER force-pushed the feature/different-screen-view branch from e90da1d to 64e09c2 Compare October 12, 2023 09:27
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@ThatcherK
Copy link
Author

  • The desktop view seems to be the same as tablet view, i would expect a difference?
  • When I click show preview , it would be good to have the current view in the preview pane highlighted, at the moment its confusing
  • Could we place the show/hide preview text on top of the toggle button, and also reduce the font size
  • Could we add the ability to expand the preview window width?

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.

4 participants