search

Sketchware-Pro / Sketchware-Pro

Sketchware Pro's sources in Java. Now anyone can contribute to Sketchware Pro.
https://sketchware.pro
Other
863 stars 254 forks source link

feat, refactor: Added Swipe to Refresh for Local Library Manager, optimizations #269

Closed khaled-0 closed 1 year ago

khaled-0 commented 2 years ago

feat: Added Swipe To Refresh Library List feat: Added Alias for libraries as an alternative for naming library Users can add or remove custom Alias for each library. If the name is too big it'll marquee.

Refactored A lot of code , specially For loops & Type Castings TODO: Look at Line 152. Either make it work or remove it. Also Merge This Reset Function To The changes.

khaled-0 commented 2 years ago

Edit: RIP Alias

Some sneak peak The name auto scrolls btw image

PranavPurwar commented 2 years ago

that alias thingy is totally unnecessary

khaled-0 commented 2 years ago

that alias thingy is totally unnecessary

that's totally very necessary. since we cant have renaming alias is very good to have

khaled-0 commented 2 years ago

I think i finished everything. ready for review

JavkhlanK commented 2 years ago

Hmm, I'm sorry to say this, but I think that feature is unnecessary, too. Why need aliases? Also, if you're really desperate to have another name for the Local library, you could unselect it on all projects and rename the folder.

khaled-0 commented 2 years ago

Hmm, I'm sorry to say this, but I think that feature is unnecessary, too. Why need aliases? Also, if you're really desperate to have another name for the Local library, you could unselect it on all projects and rename the folder.

That would be a lot of hassle instead of just being able to set some name without messing with anything. I think it's a useful feature when you have a lot of libraries. Also alias is added in library itself so every project will see it. Also I'm not forcing people to use it. Those who need it will use it.

ilyassesalama commented 2 years ago

I'd have merged the PR if only it hasn't that alias thingy.

khaled-0 commented 2 years ago

I'd have merged the PR if only it hasn't that alias thingy.

And i don't see how having it is a disadvantage. You don't have to use it if you don't like it :wtfwithtea:

omeraydindev commented 2 years ago

The argument "You don't have to use it if you don't like it" doesn't really apply when the said feature adds more complexity to the already-tedious-and-buggy-system (local libs) and possibly room for more bugs. It would've been a lot better if you sent different features in different PRs.

khaled-0 commented 2 years ago

The argument "You don't have to use it if you don't like it" doesn't really apply when the said feature adds more complexity to the already-tedious-and-buggy-system (local libs) and possibly room for more bugs. It would've been a lot better if you sent different features in different PRs.

Forget what i said.

PranavPurwar commented 2 years ago

Conclusion: Most of us disagree with that alias thing, so drop that idea (atleast for now)

hasrat-ali commented 2 years ago

I am also not endorsing this alias thingy.

khaled-0 commented 2 years ago

The commit fc72588 changes the Data Type for local_libs (per project) file to HashMap. Resulting in: Only a few IO calls compared to previous one Faster and better management. Because No need to check for duplications.

Uses OnClickListener instead of previous OnCheckChangedListener. Because OnCheckChanged gets called even when listview is scrolled, Resulting in very(enormously) buggy behavior.

khaled-0 commented 2 years ago

btw, I'm removing the alias stuff seems nobody wants/likes it

khaled-0 commented 2 years ago

Happy now!

Arashvscode commented 2 years ago

Edit: RIP Alias

Some sneak peak The name auto scrolls btw image

Wow good

ilyassesalama commented 1 year ago

Dead PR ngl. Should I close it?

hasrat-ali commented 1 year ago

Dead PR ngl. Should I close it?

Yes, it should be.

JavkhlanK commented 1 year ago

Dead PR ngl. Should I close it?

What's so bad about unmerged PRs? Are they hurting anyone?

ilyassesalama commented 1 year ago

Why would they stay there with no updates for almost a year?

JavkhlanK commented 1 year ago

Why would they stay there with no updates for almost a year?

No interest in those changes? They haven't been entirely rejected though, unlike some other e.g. useless changes. I don't see it as bad to have unmerged PRs, and no need to close if they're left untouched.

ilyassesalama commented 1 year ago

Didn't you guys not like this PR at the first place since it is "unnecessary"?

JavkhlanK commented 1 year ago

Didn't you guys not like this PR at the first place since it is "unnecessary"?

The alias feature as part of the changes were kinda unnecessary, but it was removed in a new commit.

ilyassesalama commented 1 year ago

I didn't know, I asked before closing it. But Hasrat closed it. You may reopen it if you like to.

JavkhlanK commented 1 year ago

Honestly, I don't care whether this PR is closed or not. I'd rather either use the wip version in branch wip-ManageLocalLibraryActivity, or completely rewrite the Manager. It kinda sucks right now.