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.

Sign up for GitHub

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

Jump to bottom

Hide Share button from My Projects Page #1283

Merged
merged 2 commits into from
Mar 10, 2022
Merged

Hide Share button from My Projects Page #1283

merged 2 commits into from
Mar 10, 2022

Conversation

palmtreefrb
Copy link
Contributor

@palmtreefrb palmtreefrb commented Jan 17, 2022 •
edited by megahirt
Loading

Description

Hide Share button from My Projects and other non lexicon project pages.

Fixes #1241

Type of Change

  • New feature (non-breaking change which adds functionality)
  • UI change

Screenshots

Screenshot from 2022-01-17 15-38-12
Screenshot from 2022-01-17 15-38-48
Screenshot from 2022-01-17 15-39-37

Please provide screenshots / animations for any change that involves the UI. Please provide animations to demonstrate user interaction / behavior changes

Testing on your branch

Please describe how to test and/or verify your changes. Provide instructions so we can reproduce. Please also provide relevant test data as necessary. These instructions will be used for QA testing below.

  • Share and Settings buttons are not visible on any page other than the editor page
  • Settings button is only visible in editor view. Only visible by the manager
  • When "allow others to share" is checked, all project members can see the Share button
  • When "allow others to share" is NOT checked, only the manager / owner can see the Share button

Checklist

  • I have performed a self-review of my own code
  • I have reviewed the title/description of this PR which will be used as the squashed PR commit message

qa.languageforge.org testing

Reviewers: add/replace your name below and check the box to sign-off/attest the feature works as expected on qa.languageforge.org

  • Rick (YYYY-MM-DD HH:MM)
  • Chris (YYYY-MM-DD HH:MM)

@github-actions
Copy link

github-actions bot commented Jan 17, 2022 •
edited
Loading

Unit Test Results

    1 files      1 suites   9s ⏱️
373 tests 373 ✔️ 0 💤 0 ❌

Results for commit c5a1172.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jan 18, 2022 •
edited
Loading

E2E Test Results

    1 files  47 suites   43s ⏱️
313 tests 13 ✔️ 299 💤 1 ❌
323 runs  13 ✔️ 309 💤 1 ❌

For more details on these failures, see this check.

Results for commit d0c4c07.

♻️ This comment has been updated with latest results.

@megahirt megahirt marked this pull request as ready for review January 18, 2022 09:45
megahirt
megahirt requested changes Jan 18, 2022
View reviewed changes
Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

The code looks good, and it should work as is, I'd like to ask you to make the settings and share button visibility logic the same, since they should be the same. See my comment about renaming the variable. Thank you!

src/angular-app/bellows/core/navbar.controller.ts Outdated Show resolved Hide resolved
@palmtreefrb palmtreefrb force-pushed the feature/hide-share-button-from-my-project-page branch from 9d5fa69 to d0c4c07 Compare January 20, 2022 16:18
@palmtreefrb
Copy link
Contributor Author

Added additional business logic around displaying the header buttons.

megahirt
megahirt requested changes Feb 4, 2022
View reviewed changes
Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

I recommend that have two clear booleans (or methods) that are defined in the controller, and simply reference them in the template.

src/Site/views/languageforge/container/languageforge.html.twig Outdated Show resolved Hide resolved
@megahirt megahirt force-pushed the feature/hide-share-button-from-my-project-page branch 2 times, most recently from 1416281 to 82cd457 Compare February 4, 2022 13:44
@megahirt
Copy link
Collaborator

megahirt commented Feb 4, 2022

I have rebased this branch onto the latest commit where e2e tests were still working. My hope is that I can leverage that commit for a bit longer on these strategic PRs.

@megahirt
Copy link
Collaborator

megahirt commented Feb 4, 2022

Actually, my latest changes don't work, and I've run out of time to figure out why, so I'm going to leave this PR for now. Will pick it up again sometime soon.

@megahirt megahirt self-requested a review February 4, 2022 14:23
megahirt
megahirt previously requested changes Feb 4, 2022
View reviewed changes
Copy link
Collaborator

@megahirt megahirt left a comment

Choose a reason for hiding this comment

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

Needs more work - still in progress

@megahirt megahirt force-pushed the feature/hide-share-button-from-my-project-page branch from 82cd457 to 4e8e87e Compare February 4, 2022 15:13
@megahirt megahirt force-pushed the feature/hide-share-button-from-my-project-page branch from 4e8e87e to 8bfaf45 Compare February 14, 2022 02:54
@rmunn rmunn force-pushed the feature/hide-share-button-from-my-project-page branch from f731488 to 8bfaf45 Compare February 18, 2022 02:29
@longrunningprocess longrunningprocess dismissed megahirt’s stale review February 18, 2022 02:39

The team thinks Chris might've fixed things so they're asking to dismiss while he's out

@megahirt
Copy link
Collaborator

megahirt commented Mar 10, 2022 •
edited
Loading

I have finally turned my attention back to this and was able to finish testing on my branch.

  • Share and Settings buttons are not visible on any page other than the editor page
  • Settings button is only visible in editor view. Only visible by the manager
  • When "allow others to share" is checked, all project members can see the Share button
  • When "allow others to share" is NOT checked, only the manager / owner can see the Share button

In addition, all the E2E tests are passing on my local development machine.
image

@megahirt megahirt force-pushed the feature/hide-share-button-from-my-project-page branch from 473948e to fb2885d Compare March 10, 2022 08:02
palmtreefrb and others added 2 commits March 10, 2022 15:12
@palmtreefrb @megahirt
Hide Share button from My Projects Page
2d40d0a 
Remove consol.log()

Add additional business logic to header buttons

refactor rename twig header variable param. remove console.log debug files
@megahirt
simplify logic in template / tweak E2E tests
c5a1172 
change assertion to isDisplayed()

simplify logic in template

- rename displayHeaderButtons back to displayShareButton

- displayShareButton is only for share button, not for settings button

- remove CSS app-settings-available and put logic directly in template ng-if

fix flaky test

This test asserts that the settingsMenuLink is not  present, and the appropriate test should be `isPresent()` not `isDisplayed()`
@megahirt megahirt force-pushed the feature/hide-share-button-from-my-project-page branch from fb2885d to c5a1172 Compare March 10, 2022 08:13
@megahirt megahirt merged commit 5820ae2 into develop Mar 10, 2022
@megahirt megahirt deleted the feature/hide-share-button-from-my-project-page branch March 10, 2022 08:20
@palmtreefrb
Copy link
Contributor Author

These two test failed. The buttons are still available on the "List", "Project Settings", "Synchronize", "Import Data" and "Configuration" pages.

  • Share and Settings buttons are not visible on any page other than the editor page
  • Settings button is only visible in editor view. Only visible by the manager

@alex-larkin
Copy link

alex-larkin commented Mar 24, 2022 •
edited
Loading

Great work Rick and everyone! Just to clarify, is this still being worked on? I did observe some of the behaviors you mentioned Rick. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josephmyers josephmyers josephmyers approved these changes

@megahirt megahirt megahirt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove "Share" button from My Projects Page
4 participants
@palmtreefrb @megahirt @alex-larkin @josephmyers