Feature/sup 2754 allow hiding external links #597
Conversation
-- Added Edit header to the external data cards
* dev: Updated dashboard tests SUP-2789, Edit Profile page-->3 cards should be present in one row SUP-2790, Weblink regular expression:--Able to add "http://aa" as a weblink SUP-2481, intergrate-web-links-api Remove double quotes Fixed showing top placements SUP-2757, Remove scroll listener for tc-sticky directive to avoid memory leak updated .svg assets, fixed wrong transparency
-- Removed toggle switch for hiding the profile -- Added trash icon for deleting the link -- Added unit tests for link deletion flow -- Updated unit tests for externalWeblinksService for removeLink method
-- Disabled the remove link for PENDING state card
-- Added confirmation popup before deleting the weblink -- Updated existing tests for the changes made for confirmation pop up -- Added new tests for confirmation popup functionality
-- Made the edit header visible only for weblink
-- Removed unused code
|
@parthshah @nlitwin @tladendo right now, there is some visual inconsistency in data cards because we are only showing the delete option only for weblinks. Basically, as I have to increase the height of the cards to accommodate the new header and we don't have the header for external accounts, the external account card has more white space. |
|
@vikasrohit @vic-appirio Maybe the additional whitespace problem is only temporary, we will be adding a button / link to remove external accounts soon anyway. I think it's okay to keep the additional space for now, just make sure the card design is consistent. |
| $scope.confirmDeletion = function(account) { | ||
| $scope.toDelete = account; | ||
| $scope.deletionDialog = ngDialog.open({ | ||
| className: 'ngdialog-theme-default tc-dialog', |
There was a problem hiding this comment.
Instead of passing account to be deleted & deleteAccount() on scope, provide a controller to handle this. Additional tracking with $scope.toDelete seems error prone and not "encapsulated".
https://github.com/likeastore/ngDialog#controller-string--array--object
There was a problem hiding this comment.
@parthshah I thought of the same, but reason behind not using controller for this as of now, is that I was thinking of creating a generic directive for showing a confirmation popup. May be we can use ngDialog's openConfirm itself for this. However, I was not sure, if I can style it as per our requirements and it can take some more time, hence, I chose to implement directly in external links data directive. Please let me know, if you think it would be a good idea to have a generic directive for showing the confirmation popup.
-- Updated position of trash icon to be such that it does not occupy extra height and we have consistent height of cards for both external accounts and web links. Ref: https://appirio.atlassian.net/browse/SUP-2754?focusedCommentId=20519&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-20519
|
Updated position of trash icon to be such that it does not occupy extra height and we have consistent height of cards for both external accounts and web links. |
* dev: Remove styleguide folder Begin adding submissions repo SUP-2793, 'domain.com' added web link is redirected to 'https://www.topcoder-qa.com/domain.com/' with error 'ERROR 404 : Page not Found'. Change port back to 3000 Update to use master branch for gulp-taskS Comment out tests because I cant figure out how to fix them Add bower to package.json Rename Clean up Comment out tests Add dependencies for awspublish task Change to directive module Replace gulp tasks with gulp-tasks repo Fix paths for specs.html update .version Updated month to be current re-implementing SUP-2582 changes Revert "SUP-2582, Upcoming SRMs should be ordered by most recent date" Revert "SUP-2582, Upcoming SRMs should be ordered by most recent date" Revert "SUP-2582, Upcoming SRMs should be ordered by most recent date"
* dev: commented placeholder line to allow unit tests to pass. SUP-2796, Persistently show password checkbox on mobile SUP-2796, Persistently show password checkbox on mobile
-- Implemented code review suggestion for having separate controller for account deletion logic -- Added link title to the confirmation message with italic(to differentiate it from other text) font style -- Updated unit tests after refactoring
|
@parthshah I have included the changes you suggested for wrapping the logic for actual deletion of the account into a separate controller. Unit tests are passing after the changes and functionality is working as well. :) |
|
@parthshah @nlitwin @tladendo Merging PR to get latest code in dev so that work for other issues related to the external link cards can have latest code and do not cause any conflict. However, we can still discuss any issue in the code. I have created an issue for tracking the required font in confirmation message. |
…ng-external-links Feature/sup 2754 allow hiding external links
@parthshah @nlitwin @tladendo Please provide your valuable feedback