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

Add package that displays a message for oudated browser #2872

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

randomnetcat
Copy link
Contributor

See RHCLOUD-27572.

The outdatedBrowser() call does accept an argument that tells it what browser versions to consider outdated, but I wasn't sure how to even start approaching determining that. For instance, the Patternfly documentation says the supported version for all browser is "latest", which is... less than helpful.

@randomnetcat randomnetcat added the enhancement New feature or request label Jun 24, 2024
@randomnetcat randomnetcat marked this pull request as ready for review June 24, 2024 19:27
@randomnetcat randomnetcat requested a review from a team June 25, 2024 14:22
src/outdated-browser-rework.js Outdated Show resolved Hide resolved
src/outdated-browser-rework.js Outdated Show resolved Hide resolved
config/webpack.config.js Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.66%. Comparing base (4349edb) to head (b1b7f2d).

Current head b1b7f2d differs from pull request most recent head a40fa24

Please upload reports for the commit a40fa24 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2872      +/-   ##
==========================================
+ Coverage   63.59%   63.66%   +0.07%     
==========================================
  Files         202      203       +1     
  Lines        4650     4651       +1     
  Branches      858      858              
==========================================
+ Hits         2957     2961       +4     
+ Misses       1683     1680       -3     
  Partials       10       10              
Files Coverage Δ
src/components/Footer/Footer.tsx 75.00% <ø> (ø)
src/outdated-browser.js 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

@Hyperkid123
Copy link
Contributor

/retest

@Hyperkid123
Copy link
Contributor

/retest

@Hyperkid123
Copy link
Contributor

Seems like the extensions is messing with e2e cypress/iqe tests:

image

@randomnetcat
Copy link
Contributor Author

It appears Cypress is configured to use an ancient User-Agent

// required for the redirects to work correctly due to a chromium issue
userAgent: 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36',
. Can that be changed at this point? Not sure what the "chromium issue" is.

@ryelo
Copy link
Member

ryelo commented Jun 27, 2024

We should run the content and banner by UX do double check brand standards. I know from looking the banner doesn't follow PatternFly guidelines and would like to see if they have guidance for us.

@Hyperkid123
Copy link
Contributor

@randomnetcat

Can that be changed at this point? Not sure what the "chromium issue" is.

It might be. It's very likely the lib is looking at this. its worth a shot. If that says its chrome 51 its basically an ancient version. Current is 120 or something.

@randomnetcat
Copy link
Contributor Author

Updated the userAgent. I still have a test failing locally, but it appeared to also be failing on master?

@Hyperkid123
Copy link
Contributor

/retest

There is always one run failing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants