templates/top-navigation: Update action buttons layout
ClosedPublic
Actions

Authored by anlambert on Jun 19 2020, 4:16 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T2326: Improve archived object browsing page design

Commits

rDWAPPS8e0fa87b22d7: templates/top-navigation: Update action buttons layout

Summary

Remove the Actions dropdown and replace with simple buttons instead: "Download",
"New snapshot" and "Show metadata".

Hide the "Download as git" and "Show metadata" action for non staff users.

Related to T2326

Before:

After:

Diff Detail

Repository

rDWAPPS Web applications

Branch

browse-update-action-buttons

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 12967
Build 19757: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 19756: arc lint + arc unit

Event Timeline

anlambert created this revision.Jun 19 2020, 4:16 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 19 2020, 4:16 PM

Build is green

Patch application report for D3319 (id=11757)

Rebasing onto 9358473580...

Current branch diff-target is up to date.

Changes applied before test

commit 8e0fa87b22d7196d72ab580882a61804e7c84912
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Fri Jun 19 15:28:16 2020 +0200

    templates/top-navigation: Update action buttons layout
    
    Remove the Actions dropdown and replace with simple buttons instead: "Download",
    "New snapshot" and "Show metadata".
    
    Hide the "Download as git" and "Show metadata" action for non staff users.
    
    Related to T2326

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/208/ for more details.

Harbormaster completed remote builds in B12967: Diff 11757.Jun 19 2020, 4:26 PM

Looks good.

One question above ;)

cypress/integration/vault.spec.js
24	I read in the cypress doc that it'd be best to avoid using the ui to login and use internal authentication api instead. I tried to start something about it but failed to make it ok ¯\_(ツ)_/¯ so far. I guess, we'll tackle this later. Although, note that we kinda use this snippet in multiple locations now (each time we need to login in the admin page so that'd be deposit, the main admin page test and here i guess). Maybe there we could define this as a main function we can reuse? What do you think?

This revision is now accepted and ready to land.Jun 22 2020, 9:47 AM

anlambert added inline comments.Jun 22 2020, 11:30 AM

cypress/integration/vault.spec.js
24	I read in the cypress doc that it'd be best to avoid using the ui to login and use internal authentication api instead. I tried to start something about it but failed to make it ok ¯\_(ツ)_/¯ so far. I guess, we'll tackle this later. I do not think we need to do that. The idea to go through the UI is to ensure the login process does not break as the webapp code evolves. Although, note that we kinda use this snippet in multiple locations now (each time we need to login in the admin page so that'd be deposit, the main admin page test and here i guess). Maybe there we could define this as a main function we can reuse? What do you think? Ack, will put that function in the `cypress/utils/index.js` file.

anlambert added inline comments.Jun 22 2020, 1:16 PM

cypress/integration/vault.spec.js
24	I do not think we need to do that. The idea to go through the UI is to ensure the login process > does not break as the webapp code evolves. Just found your comment while trying to factorize login process. I think I know how to do that. I will open a new diff instead of updating that one.