Skip to content

Do not add an "Add" button when modifying#122

Closed
Vinnl wants to merge 1 commit intomasterfrom
171-double-plus-ungood
Closed

Do not add an "Add" button when modifying#122
Vinnl wants to merge 1 commit intomasterfrom
171-double-plus-ungood

Conversation

@Vinnl
Copy link
Copy Markdown
Contributor

@Vinnl Vinnl commented Sep 17, 2019 •
edited
Loading

I really have no idea why these lines of code are there, but given
that they were introduced in a commit that said it was "basically"
working (2e63674), I hope @timbl can
clarify that.

Since we haven't set up test tooling in solid-ui yet, this PR does not include tests.

Fixes #121.

@Vinnl Vinnl requested review from megoth and timbl September 17, 2019 11:14
megoth
megoth suggested changes Sep 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@megoth megoth left a comment

Choose a reason for hiding this comment

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

This seems to remove the green plus icon altogether? That is at least the behavior I'm seeing when testing this out on my local setup.

@Vinnl
Copy link
Copy Markdown
Contributor Author

Vinnl commented Sep 17, 2019

Ehm, I guess you're right - let me take another look.

@Vinnl Vinnl changed the title Do not add an "Add" button when modifying WIP: Do not add an "Add" button when modifying Sep 17, 2019
@Vinnl
Do not add an "Add" button when modifying
44e4dcc 
I really have no idea when `doingDefaults` is supposed to be true,
 but given that these lines were introduced in a commit that said
it was "basically" working
(2e63674), I hope Tim can clarify
that during review. I _think_ it means when we're not rendering
folder-specific sharing settings, in which case we do not want to
add yet another "Add" button - it was already added when loading
defaults. If this commit is in `master`, it has been approved ;)
@Vinnl Vinnl force-pushed the 171-double-plus-ungood branch from bc88b1c to 44e4dcc Compare September 17, 2019 11:57
@Vinnl Vinnl changed the title WIP: Do not add an "Add" button when modifying Do not add an "Add" button when modifying Sep 17, 2019
@Vinnl
Copy link
Copy Markdown
Contributor Author

Vinnl commented Sep 17, 2019

@megoth Alright, perhaps this does what it's supposed to - can't really be sure though :/

Note that I updated the commit, so if you want to check it out locally, delete the old branch first:

$ git checkout master 
$ git branch -D 171-double-plus-ungood
$ git checkout 171-double-plus-ungood

@Vinnl Vinnl requested a review from megoth September 17, 2019 12:00
megoth
megoth approved these changes Sep 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@megoth megoth left a comment

Choose a reason for hiding this comment

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

This is limiting the number of plus-icons to one, which is good. It will only add to first list of groups, but that's ok, as you can drag-and-drop to the other one.

In my book this is a good enough solution (especially since I suspect that we will rework the sharingPane at some point anyway).

timbl
timbl reviewed Sep 18, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@timbl timbl left a comment

Choose a reason for hiding this comment

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

This looks as though it suppresses a green plus when the acl control is controlling the default settings ... not clear to me why you would not want it ... just looking on phone

@timbl
Copy link
Copy Markdown
Contributor

timbl commented Sep 18, 2019

I was expected the solution to the replicating pluses problem to be to move the plus from an outer dom level to an inner one where it vanish with as control is refreshed. ...

@timbl
Copy link
Copy Markdown
Contributor

timbl commented Sep 18, 2019

The title seems reversed. You only want a green plus when you are modifying the ACL. Not when it is read-only.

@brownhoward brownhoward added this to the Data Browser Release 4 milestone Oct 28, 2019
@megoth megoth modified the milestones: Data Browser Release 4, Data Browser Release 5 Dec 13, 2019
@megoth megoth assigned megoth and unassigned Vinnl Dec 17, 2019
@megoth
Copy link
Copy Markdown
Contributor

megoth commented Jan 9, 2020

Closed in favor of https://github.com/solid/solid-ui/pull/182

@megoth megoth closed this Jan 9, 2020
@timea-solid timea-solid deleted the 171-double-plus-ungood branch March 8, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timbl timbl timbl left review comments

+1 more reviewer

@megoth megoth megoth approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@megoth megoth

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multiple Green Pluses on Sharing Pane

5 participants

@Vinnl @timbl @megoth @timea-solid @brownhoward