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

[adoptium-510] Marketplace vendor selector should randomise order #2131

Conversation

xavierfacq
Copy link
Member

Description of change

This PR is related to the issue #510

I suggest to have the same behavior like in 'Members':

  • vendors are shuffled on display, different for all users but not when navigate (let me know if OK)
  • use the "util/shuffle" method witch is also used in "Members", and covered by tests.

Checklist

  • npm test passes

@netlify
Copy link

netlify bot commented Aug 23, 2023

Deploy Preview for eclipsefdn-adoptium ready!

Name Link
🔨 Latest commit de8534e
🔍 Latest deploy log https://app.netlify.com/sites/eclipsefdn-adoptium/deploys/64e712f30b7c210008e7e018
😎 Deploy Preview https://deploy-preview-2131--eclipsefdn-adoptium.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #2131 (de8534e) into main (fb75f63) will increase coverage by 0.00%.
Report is 7 commits behind head on main.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##             main    #2131   +/-   ##
=======================================
  Coverage   99.17%   99.17%           
=======================================
  Files          85       85           
  Lines        6274     6283    +9     
  Branches      534      536    +2     
=======================================
+ Hits         6222     6231    +9     
  Misses         52       52           
Files Changed Coverage Δ
src/components/DownloadDropdowns/index.tsx 93.47% <63.15%> (+0.29%) ⬆️
src/components/VendorSelector/index.tsx 100.00% <100.00%> (ø)
src/hooks/fetchMarketplace.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xavierfacq xavierfacq marked this pull request as ready for review August 23, 2023 13:07
@gdams
Copy link
Member

gdams commented Aug 23, 2023

This doesn't seem to be working for me. Try refreshing the page and the values get muddled up

@xavierfacq
Copy link
Member Author

xavierfacq commented Aug 23, 2023

Ok, if you do F5 or ctrl+shift+r does it work ?

Nevermind, I'll use another solution to have a good refresh.

EDIT: i think we have the same problem with the members page

@xavierfacq
Copy link
Member Author

@gdams if you can, plz test this new version, it changes every time, using useState and useEffect. There is a small resize, but I think there are problems with scss not well reloaded. I'll investigate this point, on another branch. 🙏

@tellison
Copy link
Contributor

Are you seeing entries in the table with this change? Table is empty for me.

@xavierfacq
Copy link
Member Author

@tellison can you give me your navigator / OS, and if you have an error in your console ?

I have pushed a small CSS fixe, few minutes ago, but I don't think it is the problem.

@xavierfacq xavierfacq marked this pull request as draft August 23, 2023 19:31
@xavierfacq
Copy link
Member Author

I don't understand why the last preview do not reflect what I have done... Like if there is a cache...

here : https://deploy-preview-2131--eclipsefdn-adoptium.netlify.app/marketplace/

@tellison
Copy link
Contributor

I'm on macOS Chrome Version 116.0.5845.96 (Official Build) (x86_64).
Here's a screen grab from the initial load of the page. If I explicitly choose from the drop down filters (e.g. switch to "Operating System" > Linux, then back to > macOS) the table populates correctly.

Screenshot 2023-08-23 at 20 37 43

@tellison
Copy link
Contributor

I don't understand why the last preview do not reflect what I have done... Like if there is a cache...

Yes, Netlify does cache static content on deploy preview - I've come across that before. I don't remember how to clear it manually. I think I asked @gdams to do it last time.

@xavierfacq
Copy link
Member Author

I wonder if your problem could be more a problem with the download table than the randomised vendors 🤔... You don't have the problem with the online version?
I have another idea, I'll investigate tomorrow!

@tellison
Copy link
Contributor

I wonder if your problem could be more a problem with the download table than the randomised vendors 🤔... You don't have the problem with the online version? I have another idea, I'll investigate tomorrow!

Correct - I don't see the same problem with the live site. Thanks for investigating!

@xavierfacq
Copy link
Member Author

@tellison I found the problem, and it's good now. My default list of vendors was empty to avoid the "refresh" and view the randomization. I tried another way, but it remains something strange with the page that does full refreshs.

I keep the PR as draft to test another thing.

Let me known if it's OK for you the behavior with MacOs

@tellison
Copy link
Contributor

Initial display is now populated again for me. 👍

@xavierfacq xavierfacq marked this pull request as ready for review August 24, 2023 15:00
@gdams gdams linked an issue Aug 24, 2023 that may be closed by this pull request
@gdams gdams merged commit f695cb4 into adoptium:main Aug 24, 2023
12 of 13 checks passed
@xavierfacq xavierfacq deleted the fix/adoptium-510_Marketplace_vendor_selector_should_randomise_order branch September 7, 2023 06:59
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.

Marketplace vendor selector should randomise order
3 participants