Skip to content

Conversation

@RubenVerborgh
Copy link
Contributor

I watched a new Solid user figure out how to upload stuff to a place no one could see.

Turns out it's hard/unintuitive to create this in the UI:

  • creating a child folder of / is hard, because there is an HTML page there, not Databrowser;
  • when you do create a child folder somewhere else, the "Everyone" icon is listed under Viewers, and I couldn't find how to "drag away" the icon, i.e., not give everyone any permissions.

While this PR doesnm't mitigate the above two issues, at least it provide a private folder by default, which is probably a sensible thing to do.

@RubenVerborgh RubenVerborgh force-pushed the feature/private-folder branch from 60ea133 to e92d009 Compare October 27, 2018 19:18
@RubenVerborgh RubenVerborgh removed their assignment Oct 27, 2018
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
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.

Uncertain about target="_blank", but apart from that it looks good.

<span class="lead">Public Folder</span>
<span class="badge">public</span>
</a>
<a href="/private/" target="_blank" class="list-group-item">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary with target="_blank"? It's usually an annoyance for most use cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a "Good reason" (ref https://css-tricks.com/use-target_blank/) for doing this, but maybe there's something I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy/paste from what was above.

But a good reason would be that the click brings them to Databrowser, which might be very overwhelming at first. They can just close and be back on the homepage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's continue that practice for now then

@kjetilk
Copy link
Member

kjetilk commented Nov 7, 2018

This fell off my radar... We're go to merge? In which case, it should be on the ASAP board.

@RubenVerborgh RubenVerborgh added this to the 5.0.0 milestone Nov 7, 2018
@megoth
Copy link
Contributor

megoth commented Nov 7, 2018

Yes, this could be merged; want me to rebase it on master?

@megoth
Copy link
Contributor

megoth commented Nov 7, 2018

Should be rebased and ready for merging now.

@RubenVerborgh
Copy link
Contributor Author

RubenVerborgh commented Nov 7, 2018 via email

@megoth
Copy link
Contributor

megoth commented Nov 7, 2018

Gah, my git skills are not the best, especially rebase is something I haven't quite understood yet.

But hope the result looks good nevertheless.

@RubenVerborgh
Copy link
Contributor Author

RubenVerborgh commented Nov 7, 2018 via email

@Ryuno-Ki
Copy link

Ryuno-Ki commented Nov 7, 2018

Regarding target="_blank" … came you across https://mathiasbynens.github.io/rel-noopener/ in the past?

@RubenVerborgh
Copy link
Contributor Author

RubenVerborgh commented Nov 7, 2018 via email

@RubenVerborgh RubenVerborgh force-pushed the feature/private-folder branch from d694dc3 to 0f6cd51 Compare November 7, 2018 21:51
@ghost ghost assigned RubenVerborgh Nov 7, 2018
@RubenVerborgh
Copy link
Contributor Author

@megoth For reference, here are the steps I took:

git checkout master
git pull
git checkout feature/private-folder # my local copy was still un-rebased
git rebase master
git push -f

Note how there is only 1 commit in this PR now.

@RubenVerborgh RubenVerborgh removed their assignment Nov 7, 2018
@kjetilk kjetilk changed the base branch from develop to master November 7, 2018 22:37
@kjetilk
Copy link
Member

kjetilk commented Nov 7, 2018

OK, great! We've deprecated develop, so normal merges will go to master, consistent with other repos. And then we have a released branch too.

@RubenVerborgh RubenVerborgh merged commit 8499253 into master Nov 7, 2018
@RubenVerborgh RubenVerborgh deleted the feature/private-folder branch November 7, 2018 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants