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.

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

Dart 3 class modifiers should be clearly visible in dartdocs #3392

Closed
mit-mit opened this issue Apr 21, 2023 · 14 comments · Fixed by #3401
Closed

Dart 3 class modifiers should be clearly visible in dartdocs #3392

mit-mit opened this issue Apr 21, 2023 · 14 comments · Fixed by #3401
Assignees
Labels
area-web-design Something a web designer interested in helping could do. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@mit-mit
Copy link
Member

mit-mit commented Apr 21, 2023

Dart 3 apps new class modifiers (dart-lang/language#2242), and the core libraries have been updated to many cases to use these. For example, the platform class in dart:io is now abstract final.

In the current DartDocs it's very hard to see these modifiers (e.g. platform). They are shown just textually near the top:

Screenshot 2023-04-21 at 13 39 37

I'd like to suggest the following improvements to make class modifiers more clear:

  1. Show capability chips after the class name (similar to the chips we show on API members in the rest of the page):

Screenshot 2023-04-21 at 13 43 58

  1. On generative constructors, show them as now available (e.g. using strikethrough text) with capability chips detailing why. From @lrhn the combinations where the constructor isn't available are: a) sealed, b) abstract final, and c) abstract interface).
@mit-mit
Copy link
Member Author

mit-mit commented Apr 21, 2023

cc @srawlins @jcollins-g, I think this is a pretty important change for Dart 3. wdyt?

@mit-mit
Copy link
Member Author

mit-mit commented Apr 21, 2023

We should probably also have an abstract annotation on constructors in general to clarify that they can only be called from subclasses?

@srawlins
Copy link
Member

In the current DartDocs it's very hard to see these modifiers (e.g. platform). They are shown just textually near the top:

IIRC this was an intentional decision. I don't believe we consulted UX at the time (CC @InMatrix), but some subset of the team agreed that these did not need to be prominent on the page (as abstract is not prominent).

I think we strayed away from the chips idea because @InMatrix has some students who pointed out that the chips are very prominent to the point of distraction in other pages, like this:

Screenshot 2023-04-21 at 5 55 38 AM

But in your suggestion, there wouldn't be too many chips cluttering up the page next to the class (max of... 3?) so it could probably work.

@jcollins-g
Copy link
Contributor

jcollins-g commented Apr 21, 2023

I had considered this, but this was also left out because there wasn't much time for a UX design pass when we needed to implement and ship this and given the historical handling of abstract, this seemed reasonable. This is still the case.

@jcollins-g jcollins-g added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug area-web-design Something a web designer interested in helping could do. labels Apr 21, 2023
@InMatrix
Copy link

Showing capability chips after the class name seems OK, since there aren't other chips in that part of the page. If we do that, we can drop "abstract final` from the breadcrumb to make it easier to read.

@jcollins-g
Copy link
Contributor

@InMatrix sounds reasonable. Should chips be displayed on the library pages as well with each class listed?

@mit-mit
Copy link
Member Author

mit-mit commented Apr 21, 2023

I don't think the breadcrumb is even close to visible enough. I only found it as I did a CTRL-F search for the word final.

@InMatrix
Copy link

Should chips be displayed on the library pages as well with each class listed?

It depends on what kind of user need, if any, displaying those chips on the library pages might serve. For example, do we anticipate users to look for classes with specific capabilities on a library page such as https://api.dart.dev/beta/3.0.0-417.2.beta/dart-io/dart-io-library.html? Will they decide to click into a class page based on whether that class is abstract or not? I'd not show those chips on the library pages until we identify concrete value they might provide to the user.

@mit-mit
Copy link
Member Author

mit-mit commented Apr 24, 2023

The main reason to show the modifier chips is to communicate things that you can't do (like instantiate an abstract class). I think it might be sufficient to show that on the individual class pages.

@jcollins-g jcollins-g self-assigned this Apr 24, 2023
@jcollins-g
Copy link
Contributor

I'll take a look at adding the chips and replacing the words in the breadcrumb with those. Not currently planning to cherry pick this however.

@mit-mit
Copy link
Member Author

mit-mit commented Apr 26, 2023

Why not? This seems like a major usability trap. There is no way to see in the current docs what the class capabilities are (the breadcrump is really insufficient).

@jcollins-g
Copy link
Contributor

@mit-mit Mostly because I don't think it is important enough compared to the other issues being raised for cherry picks. abstract has always been handled by the same manner and it wasn't considered urgent to fix that so it makes me wonder how urgent this really is when considered in the context of the whole language. If it wasn't for the fact that this is the shiny new feature I doubt we would be paying attention to it.

The extra effort required to either precisely cherry-pick out of a third_party repository or mitigate any risk from other PRs landing in dartdoc seems like it'd be better spent improving more things for the next release, which isn't really that far behind 3.0. I'm open to being wrong though. And if I can get this landed quickly with minimal fuss perhaps a cherry pick is possible.

@jcollins-g
Copy link
Contributor

Chatted with others about this; I will try to get a cherry pick in this week if possible, one that includes the whole set of changes since the beta cut for dartdoc including this one to mitigate the risk and effort of trying to extract individual changes. We'll also hold two large changes with higher risk that are still in the review hopper until that's done.

Should for whatever reason this change not be landed by end of week, we'll not try to last-minute cherry pick it.

@mit-mit
Copy link
Member Author

mit-mit commented Apr 26, 2023

SGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-design Something a web designer interested in helping could do. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants