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

Dashboard warns users that Safari is not supported #315

Merged
merged 2 commits into from
Aug 2, 2021
Merged

Conversation

akurinnoy
Copy link
Contributor

What does this PR do?

In this PR I added the fix that allows Dashboard to run in Safari. And since Safari is not supported users will always see the permanent banner with the warning message at the top of the page.

Screenshot 2021-08-02 at 11 40 42

What issues does this PR fix or reference?

fixes eclipse-che/che#20123

@akurinnoy akurinnoy self-assigned this Aug 2, 2021
@github-actions
Copy link

github-actions bot commented Aug 2, 2021

Docker image build succeeded: docker.io/maxura/che-dashboard:che-dashboard-pull-315

@che-bot
Copy link
Contributor

che-bot commented Aug 2, 2021

✅ E2E dashboard tests succeed 🎉

See Details

Test product:

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #315 (6d7d692) into main (b90da0b) will decrease coverage by 0.03%.
The diff coverage is 37.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   52.10%   52.07%   -0.04%     
==========================================
  Files         146      148       +2     
  Lines        5617     5640      +23     
  Branches      921      923       +2     
==========================================
+ Hits         2927     2937      +10     
- Misses       2413     2426      +13     
  Partials      277      277              
Impacted Files Coverage Δ
...oard-frontend/src/components/BannerAlert/index.tsx 0.00% <0.00%> (ø)
...ges/dashboard-frontend/src/store/Branding/index.ts 0.00% <0.00%> (ø)
...mponents/BannerAlert/NotSupportedBrowser/index.tsx 100.00% <100.00%> (ø)
...ard-frontend/src/services/helpers/detectBrowser.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b90da0b...6d7d692. Read the comment docs.

Copy link
Contributor

@olexii4 olexii4 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM

Since officially we don't support any other than Google Chrome, it makes sense to show it on the Firefox as well.

Screenshot_20210802_144045

but it out of the current PR scope. I'll create a dedicated issue and raise it on Che Community Call.

@openshift-ci
Copy link

openshift-ci bot commented Aug 2, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, olexii4, sleshchenko

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sleshchenko
Copy link
Member

please, after merging create PR with cherry-pick the stuff onto 7.34.x

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

Successfully merging this pull request may close these issues.

Dashboard should report that it does not support Safari
4 participants