Page MenuHomeSoftware Heritage
Differential D7428

Save Code Now: Use different views for each tab
ClosedPublic
Actions

Authored by vlorentz on Mar 25 2022, 10:46 AM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Commits
rDWAPPS72b5dd24dc3f: Save Code Now: Use different views for each tab
Summary

This is IMO more manageable than handling it manually with URL fragments
(which repeatedly breaks when adding new tabs).

If you are fine with this change, I will do it for Add Forge Now
and the admin interface, too.

Diff Detail

Repository
rDWAPPS Web applications
Branch
tabs
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27800
Build 43516: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 43515: arc lint + arc unit

Event Timeline

vlorentz created this revision.Mar 25 2022, 10:46 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 25 2022, 10:46 AM
vlorentz edited the summary of this revision. (Show Details)Mar 25 2022, 10:47 AM
vlorentz edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Mar 25 2022, 11:08 AM

Build was aborted

Patch application report for D7428 (id=26883)

Rebasing onto d5e3f98417...

Current branch diff-target is up to date.
Changes applied before test
commit 444cb026f79e82db2b4baea8c0530a2e336b6853
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1572/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1572/console

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 25 2022, 11:08 AM
Harbormaster failed remote builds in B27796: Diff 26883!
vlorentz updated this revision to Diff 26886.Mar 25 2022, 11:09 AM

re-add id attribute, needed by cypress tests

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 25 2022, 11:10 AM
Harbormaster failed remote builds in B27799: Diff 26886!
vlorentz updated this revision to Diff 26887.Mar 25 2022, 11:12 AM

adapt cypress tests to work with page reload

swh-public-ci added a comment.Mar 25 2022, 11:29 AM

Build has FAILED

Patch application report for D7428 (id=26886)

Rebasing onto 349c7e5f4e...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit 6eb30715a1c4670fd1b23e906b3d499c83e8702b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1573/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1573/console

swh-public-ci added a comment.Mar 25 2022, 11:32 AM

Build is green

Patch application report for D7428 (id=26887)

Rebasing onto 349c7e5f4e...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit 7e9651a8c91327111b9cbad4eb57d8c9f7f93eb3
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

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

Harbormaster completed remote builds in B27800: Diff 26887.Mar 25 2022, 11:32 AM
vlorentz requested review of this revision.Mar 25 2022, 11:32 AM
anlambert requested changes to this revision.Mar 25 2022, 12:06 PM
anlambert added a subscriber: anlambert.

I am not against it but we should add a redirection for the old URL.

Also the CSS seems broken, top margin is no longer effective.

swh/web/templates/misc/origin-save.html
73–76

You can set active tab by relying of the view name instead:

<ul class="nav nav-tabs" style="padding-left: 5px;">
  <li class="nav-item">
    <a class="nav-link {% if request.resolver_match.view_name == 'origin-save' %}active{% endif %}"
        href="{% url 'origin-save' %}" id="swh-origin-save-request-help-tab">Help
    </a>
  </li>
  <li class="nav-item">
    <a class="nav-link {% if request.resolver_match.view_name == 'origin-save-list' %}active{% endif %}"
       href="{% url 'origin-save-list' %}" id="swh-origin-save-requests-list-tab">Browse save requests
    </a>
  </li>
</ul>
This revision now requires changes to proceed.Mar 25 2022, 12:06 PM
vlorentz added inline comments.Mar 25 2022, 1:43 PM
swh/web/templates/misc/origin-save.html
73–76

nice

vlorentz updated this revision to Diff 26896.Mar 25 2022, 1:44 PM

fix margin + use view names in template

anlambert added inline comments.Mar 25 2022, 1:58 PM
assets/src/bundles/save/index.js
249

We should add a redirection for backward compatibility with the old URL, something like:

if (window.location.pathname == Urls.origin_save() && window.location.hash == '#requests') {
  window.location = Urls.origin_save_list();
}
swh/web/templates/misc/origin-save-help.html
11–51

You need to wrap that code in a div with class tab-pane.

swh/web/templates/misc/origin-save-list.html
11

You also miss to add the tab-pane class.

swh-public-ci added a comment.Mar 25 2022, 2:03 PM

Build has FAILED

Patch application report for D7428 (id=26896)

Rebasing onto 1ffff63f70...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit d7f8c86e93203695d3f831244c4bf71378a3e9cc
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1582/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1582/console

Harbormaster failed remote builds in B27810: Diff 26896!Mar 25 2022, 2:03 PM
vlorentz updated this revision to Diff 26903.Mar 25 2022, 2:59 PM

re-add clases + add redirect

assets/src/bundles/save/index.js
249

nice, thanks

swh-public-ci added a comment.Mar 25 2022, 3:01 PM

Build has FAILED

Patch application report for D7428 (id=26903)

Rebasing onto cdda03c3aa...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit 36f68c284c8e3c9493bb3a28af3d052cf59fb797
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1584/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1584/console

Harbormaster failed remote builds in B27817: Diff 26903!Mar 25 2022, 3:01 PM
vlorentz updated this revision to Diff 26906.Mar 25 2022, 3:26 PM

fix mypy deps

vlorentz updated this revision to Diff 26908.Mar 25 2022, 3:26 PM

oops, wrong diff

Harbormaster failed remote builds in B27821: Diff 26908!Mar 25 2022, 3:27 PM
Harbormaster failed remote builds in B27819: Diff 26906!
swh-public-ci added a comment.Mar 25 2022, 3:28 PM

Build has FAILED

Patch application report for D7428 (id=26906)

Rebasing onto 4144ac789f...

Current branch diff-target is up to date.
Changes applied before test
commit ce497b9800818cf5929301a4a6f174a019120038
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Mar 23 13:49:19 2022 +0100

    Add OriginLocator, to list candidate origins to recover objects from

Link to build: https://jenkins.softwareheritage.org/job/DSCRUB/job/tests-on-diff/24/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCRUB/job/tests-on-diff/24/console

swh-public-ci added a comment.Mar 25 2022, 3:49 PM

Build was aborted

Patch application report for D7428 (id=26908)

Rebasing onto cdda03c3aa...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit 0868127e189fae73a491e0fb60c40604cf1d6316
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1586/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1586/console

swh-public-ci added a comment.Mar 28 2022, 10:54 AM

Build has FAILED

Patch application report for D7428 (id=26908)

Rebasing onto 6b5aa57b3f...

First, rewinding head to replay your work on top of it...
Applying: Save Code Now: Use different views for each tab
Changes applied before test
commit d1b73b1f449719d3acc7ccb719c9448c28f1b327
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1595/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1595/console

vlorentz mentioned this in D7439: Add Forge Now: Use different views for each tab.Mar 28 2022, 11:41 AM
vlorentz updated this revision to Diff 26939.Mar 28 2022, 12:32 PM

rebase

swh-public-ci added a comment.Mar 28 2022, 12:34 PM

Build has FAILED

Patch application report for D7428 (id=26939)

Rebasing onto 6b5aa57b3f...

Current branch diff-target is up to date.
Changes applied before test
commit 84c22e278a3437beb06c8969d860c027cfd37f70
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1597/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1597/console

Harbormaster failed remote builds in B27856: Diff 26939!Mar 28 2022, 12:34 PM
ardumont added a subscriber: ardumont.Mar 28 2022, 5:17 PM

This probably needs a rebase with @anlambert's fix so the build has a chance to succeed.

vlorentz updated this revision to Diff 26961.Mar 29 2022, 10:27 AM

rebase

swh-public-ci added a comment.Mar 29 2022, 10:50 AM

Build was aborted

Patch application report for D7428 (id=26961)

Rebasing onto 4bc8aaf1d9...

Current branch diff-target is up to date.
Changes applied before test
commit 72b5dd24dc3f783c702c47e36fd7a3ecd95b594c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Mar 25 10:46:29 2022 +0100

    Save Code Now: Use different views for each tab
    
    This is IMO more manageable than handling it manually with URL fragments
    (which repeatedly breaks when adding new tabs).
    
    A future commit will do this for Add Forge Now, too.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1610/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1610/console

Harbormaster failed remote builds in B27877: Diff 26961!Mar 29 2022, 10:50 AM
anlambert accepted this revision.Mar 29 2022, 11:45 AM

Looks good to me.

This revision is now accepted and ready to land.Mar 29 2022, 11:45 AM
This revision was landed with ongoing or failed builds.Mar 29 2022, 1:14 PM
Closed by commit rDWAPPS72b5dd24dc3f: Save Code Now: Use different views for each tab (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDWAPPS72b5dd24dc3f: Save Code Now: Use different views for each tab.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
assets/
src/
bundles/
save/
12 lines86%
Loading...
cypress/
integration/
4 lines--
swh/
web/
misc/
22 lines81%
Loading...
templates/
misc/
53 lines--
26 lines--
67 lines--

Diff 26887

View Options

assets/src/bundles/save/index.js

Loading...
View Options

cypress/integration/origin-save.spec.js

Loading...
View Options

swh/web/misc/origin_save.py

Loading...
View Options

swh/web/templates/misc/origin-save-help.html

Loading...
View Options

swh/web/templates/misc/origin-save-list.html

Loading...
View Options

swh/web/templates/misc/origin-save.html

Loading...
About · Developer information · Legal